-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
fix(dashboard): Remove bar at bottom of dashboard edit sidebar #21807
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21807 +/- ##
==========================================
- Coverage 66.95% 66.95% -0.01%
==========================================
Files 1807 1807
Lines 69196 69186 -10
Branches 7402 7399 -3
==========================================
- Hits 46331 46322 -9
- Misses 20954 20955 +1
+ Partials 1911 1909 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
b5b893a
to
1fce4be
Compare
/testenv up |
@stephenLYZ Ephemeral environment spinning up at http://35.91.193.56: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.
Awesome work! I have found some issues:
- It looks like layout elements tab has incorrect width:
master | current |
---|---|
- The shadows on the left do not behave the same as the current master, is this as expected?
master | current |
---|---|
- It is wired that tab shakes when searching
current:
2022-10-15.11.15.47.mov
master:
2022-10-15.11.17.35.mov
@stephenLYZ Oh jeez sorry, those first two were things I thought I fixed and had no idea about the last one – should have double-checked before pushing. Will fix those ASAP. |
/testenv up |
@stephenLYZ Ephemeral environment spinning up at http://34.213.209.72:8080. Credentials are |
It seems weird that the entire page shakes when searching charts. Maybe cause by re-rendering: 2022-10-21.6.48.31.mov |
@stephenLYZ Yikes that is strange, thanks for catching. Will see if I can fix. |
c891568
to
16555ff
Compare
@stephenLYZ Was finally able to reproduce by making my screen very very large! I couldn't figure out exactly what was causing it but I narrowed it down to something about the Before: Before.movAfter: After.mov |
/testenv up |
@stephenLYZ Ephemeral environment spinning up at http://18.236.139.253: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.
LGTM! Thanks for this!
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This PR removes a bar that was appearing at the bottom of the dashboard edit sidebar. It does some misc cleanup along the way:
builder-sidepane.less
, which contains a few actively-used styles but is mostly unused styles, and moves the actively-used styles into the relevant component filesreact-sticky
with CSSposition: sticky
(which seems to now be supported for all of the browsers we're targeting) for the sidebarBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
standalone=1
to the end of the query string)ADDITIONAL INFORMATION