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

Set of improvements and refinements to visualizations after React migration #4382

Merged

Conversation

kravets-levko
Copy link
Collaborator

@kravets-levko kravets-levko commented Nov 21, 2019

What type of PR is this? (check all applicable)

  • Refactor

Description

Related Tickets & Documents

#3301

@kravets-levko kravets-levko force-pushed the visualizations-refinements-after-react-migration branch from 010d998 to 70b9dc8 Compare November 21, 2019 18:19
@kravets-levko kravets-levko force-pushed the visualizations-refinements-after-react-migration branch from d98a467 to 3027f29 Compare November 22, 2019 10:34
@kravets-levko kravets-levko force-pushed the visualizations-refinements-after-react-migration branch from a82255a to 0ed194c Compare November 22, 2019 23:45
@kravets-levko kravets-levko force-pushed the visualizations-refinements-after-react-migration branch from 0ed194c to 94e5ac1 Compare November 23, 2019 08:39
@kravets-levko kravets-levko marked this pull request as ready for review November 23, 2019 10:17
@kravets-levko
Copy link
Collaborator Author

@arikfr @gabrieldutra I touched a lot of files, but I tried to do one change per commit, so you can check commits one by one to see what I did here.

@arikfr
Copy link
Member

arikfr commented Nov 24, 2019

All the help links that refer to redash.io/help, should just open the HelpDrawer -- no need to have the Popover with the link. For clarity we can add a tooltip saying "Formatting Help".

Copy link
Member

@gabrieldutra gabrieldutra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code + Percy changes look good to me 👌

@kravets-levko
Copy link
Collaborator Author

@arikfr I updated help icon for number formats - now click on it will open help drawer immediately (also added a tooltip):
image

Also, I updated help icon for date/time formats to match number format one - it's just a link that opens moment.js help:
image

Other context help blocks are unchanged:
image

Also, there are few more places with redash.io links (e.g. empty state components) - do you think they should also open a help drawer? If yes - I think it's better to do it in a new PR anyway.

@kravets-levko
Copy link
Collaborator Author

@arikfr If you're fine with this PR - I'd like to merge it.

Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might want to update the tooltips, but no reason to wait with this PR for it. Let's merge.

@kravets-levko kravets-levko merged commit 94bd03d into master Dec 4, 2019
@kravets-levko kravets-levko deleted the visualizations-refinements-after-react-migration branch December 4, 2019 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants