-
Notifications
You must be signed in to change notification settings - Fork 2
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
Track in provenance graph thegroupingChanged
LineUp event
#385
Conversation
Previously the event listener was added to event + `Changed.track` but removed from event + `Changed.filter`. This commit corrects and unifies the event name.
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.
Please add some unit tests for the util functions.
* The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. | ||
* @param groupBy Value returned from the `Group By` dialog | ||
*/ | ||
export function serializeGroupByValue(groupBy: IStringGroupCriteria) { |
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.
Could you please add some unit tests for this function?
* Restores LineUp StringColumn's Group By` dialog's values from the provenance graph and returns an `IStringGroupCriteria`. | ||
* @param groupBy Value as saved in the provenance graph. | ||
*/ | ||
export function restoreGroupByValue(groupBy: ISerializedStringGroupCriteria) { |
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.
Could you please add some unit tests for this function?
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.
@thinkh I added test for the serialize and restore functions. You can take a look again when you have time.
…ng_column_group_by_cmd_bug # Conflicts: # src/lineup/internal/cmds.ts
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.
@oltionchampari Thanks for the additional unit tests! Now it looks good. 👍
Closes #379
Summary
Set Property grouping
.The events
StringColumn.EVENT_GROUPING_CHANGED
andRanking.EVENT_GROUP_CRITERIA_CHANGED
,never intersect apart from when the groupBy dialog is set for the first time.
Screenshot