-
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: refactor header menu to show in header grid component #16689
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16689 +/- ##
=======================================
Coverage 76.88% 76.89%
=======================================
Files 1005 1005
Lines 54005 54061 +56
Branches 7337 7376 +39
=======================================
+ Hits 41522 41568 +46
- Misses 12243 12253 +10
Partials 240 240
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
May I know why do you want to remove extra UI menu/options for dashboard header? Dashboard header and markdown component do have extra UI menu which are not available fro chart. But why is it an issue? |
@graceguo-supercat, we want to make this change because of 2 reasons: We got this feedback from a first time user feeback so we suggest that |
based on your comment, this PR didn't do it correctly. it removed font-size and background menu, is it part of design? |
/testenv up |
@graceguo-supercat font size and background menu is still there. You have to click on the grid component to have them show like you do in markdown. |
@pkdotson Ephemeral environment spinning up at http://54.245.135.47:8080. Credentials are |
@pkdotson I tested in the test env, for header, it is worked as expected: trash icon is showed when hovering over, and the font size and background menu is shown when clicking on the component. But for markdown, the trash icon is not shown inside the component when hover over, it is still shown by clicking. Screen.Recording.2021-09-14.at.10.13.04.AM.mov |
@graceguo-supercat I tried in the test env, the font-size and background menu will show by click in header. Is our plan to change the trashcan to show by hovering over in header and markdown sound good to you? |
before: (both header and markdown) click to show UI menu + trash icon after: isn't this more inconsistent? Do you really need to make this change? |
@graceguo-supercat this has been changed as well. The delete now shows in the markdown on hover as well. |
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.
LGTM! thanks for the work!
/testenv up |
@pkdotson Ephemeral environment spinning up at http://35.85.65.49:8080. Credentials are |
Ephemeral environment shutdown and build artifacts deleted. |
…16689) * initial commit * add delete to markdown
…16689) * initial commit * add delete to markdown
SUMMARY
This pr removes the hovermenu less file and refactors code to show the dashboard hover menu delete icon inside the header.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
before
Screen.Recording.2021-09-13.at.8.49.48.AM.mov
after
Screen.Recording.2021-09-13.at.8.50.43.AM.mov
TESTING INSTRUCTIONS
Go to a dashboard and click on edit. Next drag a header tab to dashboard and ensure that the delete button show up on hover.
ADDITIONAL INFORMATION