Skip to content
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

Attempt to Improve Selector Support for Tooltip Handler #37

Closed
dm-p opened this issue Apr 29, 2021 · 3 comments
Closed

Attempt to Improve Selector Support for Tooltip Handler #37

dm-p opened this issue Apr 29, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dm-p
Copy link
Member

dm-p commented Apr 29, 2021

With changes in #21 implemented, we can potentially create finer-grained selectors for column/measure combinations. This means that we could also potentially attempt to create selectors for layers containing transforms away from the grain of the data view.

We should add-in functionality to the handler (in the absence of the in-built __key__ field in the datum) to attempt to create a selector for partial fields, if they can be identified by name/value in the data view. This should then resolve report page tooltips accordingly.

This will not catch all cases, but will enhance the number of scenarios we can apply report page tooltips to. This should also set us in good stead for figuring out #6 and #5.

@dm-p dm-p added this to the 0.4 milestone Apr 29, 2021
@dm-p dm-p self-assigned this Apr 29, 2021
@dm-p dm-p added the enhancement New feature or request label May 19, 2021
dm-p added a commit that referenced this issue Jun 9, 2021
- selection API refactored and extended to attempt dynamic creation selection IDs based on datum keys and values for tooltips
- tooltip API extended to use dynamic selection ID attempts for tooltips, which extends scenarios where report page tooltips may be possible to resolve based on datum alone
- categories added to store so that they can be leveraged post-update
- DataViewService and store refactored to use API methods rather than repeating code, and helping to consolidate it
- dataset and store APIs added to assist with selection API changes
- dataset API-specific interfaces moved under api from types catch-all
- APIs re-shuffled to suit required changes for dynamic selection IDs
- Refactored unit tests for revised API changes (tooltip no longer works for reasons I can't fathom. Suspect a mock is needed but can't find the right thing to mock; have replaced with a dummy test, which I'm not too happy about... but never mind)

Will need to do more testing and verification to confirm issue is actually done but as it's a lot of changes, I'd like to start the next work from a fresh commit.
@dm-p
Copy link
Member Author

dm-p commented Jun 9, 2021

Changes in build 90 catch all my current use cases, so we'll close this for now and address any new cases as they come up.

@dm-p dm-p closed this as completed Jun 9, 2021
@dm-p dm-p reopened this Jun 30, 2021
@dm-p
Copy link
Member Author

dm-p commented Jun 30, 2021

I'm finding some good cases as part of an upcoming demo where this is not fully working, so we'll have another look.

@dm-p
Copy link
Member Author

dm-p commented Jul 5, 2021

On further investigation and review, the current logic is doing what it should. Issues appear to be when the combination of field + value for a datum cannot be found in the data view for matched metadata.

This could be problematic where we've say, aggregated a column to a higher level of granularity, and the measure value is therefore summarised to something else (e.g. total, or filtered total). It's not easy for us to understand the context here, as we don't really have full detail from the datum as to how it might be filtered or aggregated. Whereas, if we were managing through a UI, we could likely prescribe the steps to filter the data view and create the correct combination.

A good example of this is the marginal bars demo, where we concatenate aggregated plots for each series:

image

Individual 'cells' work, as these represent an individual data point. however, the metadata matcher will find the appropriate column for each subplot but not find the corresponding measure value. In this case, we could perhaps assume that all 12 rows should be built into the selection ID, or we can just leave it out, but it may not be correct for other cases.

I think that for now, this might be a good compromise and we can raise another issue for continued improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant