-
Notifications
You must be signed in to change notification settings - Fork 914
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
[BUG][Console] Fix updating of play and wrench button #3920
Conversation
8bd1ee1
to
f454bb6
Compare
Fix UI bug that did not update the console with the correct values. offsetTop was always 0 and the value needed the pixel. Issue: opensearch-project#3916 Signed-off-by: Kawika Avilla <[email protected]>
f454bb6
to
8dfd67e
Compare
links failing from things not added. |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #3920 +/- ##
==========================================
+ Coverage 66.38% 66.43% +0.05%
==========================================
Files 3227 3227
Lines 62051 62051
Branches 9593 9593
==========================================
+ Hits 41191 41223 +32
+ Misses 18553 18526 -27
+ Partials 2307 2302 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@kavilla Was this a regression from one of the recent changes to the console or a latent bug? If it's a regression, it would be nice to link the PR here for context. Also, if you have a little animation of the fixed behavior, that'd be 🥇 . |
ah, yeah, this is the PR: #3733 |
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.
This looks good, but I don't think we need a release note - it's just a fix/continuation of the jquery removal PR already included.
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
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.
This needs tests.
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.
This fix is not sufficient. Instead, we should revert b7af56f, because autocompletion and documentation links are also broken with that commit.
Closing for #3929 |
Description
Fix UI bug that did not update the console with the correct values.
offsetTop was always 0 and the value needed the pixel.
Issues Resolved
closes: #3916
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr