-
Notifications
You must be signed in to change notification settings - Fork 13.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
chore(newchart): update chart creation dataset selection help text, styles #20369
chore(newchart): update chart creation dataset selection help text, styles #20369
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20369 +/- ##
==========================================
- Coverage 66.70% 66.56% -0.15%
==========================================
Files 1739 1738 -1
Lines 65148 65119 -29
Branches 6897 6899 +2
==========================================
- Hits 43457 43345 -112
- Misses 19938 20020 +82
- Partials 1753 1754 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@codyml I think adding an option to disable the cache is not the optimal solution. Especially because users will not create new datasets most of the time. A better solution is to provide a way for invalidating the cache when necessary, giving more control to the developers. We can use the |
5b460da
to
cb38b1f
Compare
@michael-s-molina Re: opening the + Dataset modal via URL: I tried changing the trigger from hash |
Can we maybe make the "Add dataset" modal reusable and open the modal directly from the "Add a dataset" link here? Also, it seems the current "Add a dataset" modal supports only adding a physical dataset. We should probably design a way for users to create virtual datasets here, too. |
That all sounds reasonable to me, but from what I can tell the |
cb38b1f
to
8198868
Compare
Unless there is immediate urgency in adding this feature right now, I'd say we should implement the optimal user experience in one go instead of shipping something we'd likely to change again very soon. The redesign of the add dataset model should definitely be future work, but the reusing the modal shouldn't be that difficult as there are very limited number of input fields in it... |
@ktmud After some digging it looks like bringing the |
8198868
to
dfe7eb0
Compare
/testenv up |
@michael-s-molina Ephemeral environment spinning up at http://34.221.182.114:8080. Credentials are |
I think it's ok to use the hash if we consider the modal as a subordinate of the listing. |
0818031
to
bd35ee5
Compare
…and only include an 'Add dataset' link when user has permission to add dataset.
bd35ee5
to
33090fe
Compare
/testenv up |
@michael-s-molina Ephemeral environment spinning up at http://34.222.96.94:8080. Credentials are |
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.
Thanks for the work @codyml! As agreed, we'll work on syncing the Select dataset and opening the modal in place in a follow-up PR after the SPA work. I left some non-blocking suggestions.
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Ephemeral environment shutdown and build artifacts deleted. |
…tyles (apache#20369) * Update dataset selection help text. * Update 'Create a new chart' flow styles. * Add support for linking directly to Create Dataset modal via URL hash. * Add support for linking directly to Create Dataset modal via URL hash. * Update dataset help text to not include spaces in translated strings and only include an 'Add dataset' link when user has permission to add dataset. * Clean up test file Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
SUMMARY
#create
URL hash on the dataset CRUD page that opens the + Dataset modal at loadBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
/chart/add
and open the "Choose a dataset" select./tablemodelview/list
in a new tab with the "Add dataset" modal open.ADDITIONAL INFORMATION