-
Notifications
You must be signed in to change notification settings - Fork 867
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 (discover): Fix dismiss callout and next styling #4938
Fix (discover): Fix dismiss callout and next styling #4938
Conversation
1. Fix `onDismiss` prop of callout 2. Put callout in panel for better alignment in discover 3. Add `subdued` to combobox panel 4. Use `primary`-derived color for field titles in discover (and lint-ignore) Signed-off-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Backporting for 2.10 right? |
fix conflict
@@ -49,11 +49,27 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro | |||
if (isCallOutVisible) { | |||
callOut = ( | |||
<EuiFlexItem grow={false}> | |||
<EuiPanel hasBorder={false} hasShadow={false} color="transparent" paddingSize="s"> |
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.
Why do we have 2 callouts here?
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.
that was a merge conflict - should be fixed
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.
oh, somehow managed to not commit - let me try again.
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.
ha, OK, yeah, this should be fixed - but doublecheck my resolution here.
Codecov Report
@@ Coverage Diff @@
## main #4938 +/- ##
=======================================
Coverage 66.39% 66.39%
=======================================
Files 3398 3398
Lines 64815 64815
Branches 10362 10362
=======================================
Hits 43033 43033
+ Misses 19221 19198 -23
- Partials 2561 2584 +23
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: Josh Romero <rmerqg@amazon.com>
47b0b26
@joshuarrrr looks like it just needs a snapshot updating |
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Will set DCO to pass, it's being squashed anyways. |
* Fix (discover): Fix dismiss callout and next styling 1. Fix `onDismiss` prop of callout 2. Put callout in panel for better alignment in discover 3. Add `subdued` to combobox panel 4. Use `primary`-derived color for field titles in discover (and lint-ignore) Signed-off-by: Josh Romero <rmerqg@amazon.com> * Update index.tsx fix conflict * fix linting indentation Signed-off-by: Josh Romero <rmerqg@amazon.com> * update snapshot Signed-off-by: Josh Romero <rmerqg@amazon.com> --------- Signed-off-by: Josh Romero <rmerqg@amazon.com> (cherry picked from commit d3388c5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Fix (discover): Fix dismiss callout and next styling 1. Fix `onDismiss` prop of callout 2. Put callout in panel for better alignment in discover 3. Add `subdued` to combobox panel 4. Use `primary`-derived color for field titles in discover (and lint-ignore) * Update index.tsx fix conflict * fix linting indentation * update snapshot --------- (cherry picked from commit d3388c5) Signed-off-by: Josh Romero <rmerqg@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Fixes some small styling issues identified by @KrooshalUX and @kgcreative when reviewing
next
themingonDismiss
prop of calloutsubdued
to combobox panelprimary
-derived color for field titles in discover (and lint-ignore)Note - I tried to fix the histogram rect highlight color in
next
dark mode, but can't figure out the problem. I got as far as confirming that bothdiscover
andlegacy_discover
pass the same styling props, but somehow it's not applied the same in each case.I also notices some other minor fit and finish issues in the video:
Issues Resolved
Screenshot
Screen.Recording.2023-09-05.at.3.48.24.PM.mov
Testing the changes
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr