Use description column as name from CommCareHQ #1
Conversation
| @@ -211,6 +211,10 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({ | |||
| Header: t('Name'), | |||
| accessor: 'table_name', | |||
| }, | |||
| { | |||
| Header: t('Name in CommCare HQ'), | |||
There was a problem hiding this comment.
If we label this "Description", do you think there is a chance of this getting merged into Apache Superset?
I agree that "Name in CommCare HQ" would be better for our users, because it would make it clear what's happening here. On the other hand, anything we can get merged would reduce our long-term maintenance burden.
What do you think? Should we try to get it merged? Should he keep this branch, and have a second branch with "Description" and try to get that one merged, just to see whether it gets accepted?
There was a problem hiding this comment.
The reason, I thought that the Superset team won't merge this in is because the description is already displayed when you hover an icon next to the name.
I think Description is meant to be displayed when you are on that object only, but I guess we could try, I will raise a PR (with 'Description') and see what they say.
There was a problem hiding this comment.
Ah! That's a very good point.
If it was me reviewing this PR from their perspective, I'd want to drop the tooltip if there was a column that showed the same information.
And they probably put some thought into the choice of a tooltip over another column, maybe to support long descriptions that don't either get cut off or squash up the other columns. Yeah, I think you're right -- I wouldn't put much hope in getting this merged.
There was a problem hiding this comment.
Yeah!
I'd want to drop the tooltip if there was a column that showed the same information.
The reason I avoided this was to minimize the code changes in the fork, so that it's easier to maintain the fork in future. (the more changes in the fork, the more surface area for merge conflicts in future). After you mentioned, I did try that now and found that I will have to change a lot of code to do that, so may be it's good to leave as is?
There was a problem hiding this comment.
I totally agree. I'm happy with what you've done here. The smaller the surface area the better.
@kaapstorm FYI