-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[I# 1011] Second implementation of facet renaming #5759
base: master
Are you sure you want to change the base?
[I# 1011] Second implementation of facet renaming #5759
Conversation
This reverts commit b2da495.
Return human string instead of `<core-views/non-blank-values>` in the ToolTip
Use label to get Facet name.
@wetneb : I've changed the Source Did I understand your reservation in the meeting? Does this address it? This should be finished, all tests passed. Regards, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but the following adjustments should be made:
Change 'Click here to edit the name of the facet' to 'Click to rename facet' so it is consistent with rename project verbiage:
There is no need to put the expression in the rename facet tooltip. The expression is visible in the 'change' tooltip, or by clicking 'change' to popup the expression dialog:
"filter(row.columnNames,cn,isNonBlank(cells[cn].value))": | ||
{ source: "<non-blank-values>", label: 'core-views/non-blank-values' }, | ||
"filter(row.columnNames,cn,isNonBlank(if(row.record.fromRowIndex==row.index,row.record.cells[cn].value.join(\"\"),null)))": | ||
{ source: "<non-blank-records>", label: 'core-views/non-blank-records' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather store this mapping in the definition of the facets themselves (in the menu). Because it is likely that people forget to update this when they add a new facet in that menu, or for instance when an extension registers a facet there. I think this could be done simply changing the method which adds a new facet, so that the facet name can be set directly there.
Fixes #1011
Changes proposed in this pull request: