[Global search bar] Fix clear button glitch #137251

gsoldevila · 2022-07-27T08:02:46Z

Fixes #135529

The PR fixes 2 glitches:

Removing the 400px => 600px expand animation on focus (which shrinks back to 400px on blur). The blur event happens before the click, so we're actually losing the click on the clear button (plus layout shift on user action is usually something we want to avoid).
- Introduced by Small fixes to Kibana search bar #85079
Hiding the append button (⌘/ shortcut hint) if the user has already entered some text. This is also a source of layout shift that causes the user to miss the clear button on click.
- Introduced by [Global Search Bar] Updated placeholder text to be more directive #127903

See discussion on #135529 for more information.

@elastic/kibana-design can you confirm you are okay with these changes?

elasticmachine · 2022-07-27T08:03:43Z

Pinging @elastic/kibana-core (Team:Core)

gsoldevila · 2022-07-27T09:06:42Z

Search bar behaviour after the fix:

Screen.Recording.2022-07-27.at.11.04.17.mov

cchaos

@gsoldevila Thanks for making the PR. A couple of quick comments.

x-pack/plugins/global_search_bar/public/components/search_bar.scss

cchaos · 2022-07-27T12:56:32Z

x-pack/plugins/global_search_bar/public/components/search_bar.tsx

        },
        fullWidth: true,
        append: showAppend ? (
          <EuiFormLabel
            title={keyboardShortcutTooltip}
-            css={{ fontFamily: euiTheme.font.familyCode }}
+            css={{ fontFamily: euiTheme.font.familyCode, position: 'absolute', right: 0 }}


Why is the absolute positioning necessary?

For context, we are adding / removing the component onBlur and onFocus respectively, so that the shortcut hint is only visible when the search bar is not already focussed.

The absolute positioning is necessary so that when removing / adding back the append component it does not affect the parent container's size.

This way we completely avoid a small layout shift that is visually disturbing.

Behaviour without the absolute positioning (you can compare with the video demoing the complete fix, above):

Screen.Recording.2022-07-27.at.15.27.01.mov

This is because the width is being applied at the search input instead of the overall wrapping element. I'll describe the fix a review comment.

Then you can delete this absolute positioning.

Suggested change

css={{ fontFamily: euiTheme.font.familyCode, position: 'absolute', right: 0 }}

css={{ fontFamily: euiTheme.font.familyCode }}

cchaos

Move line 295 where the className is set within searchProps to the top level component, ~line 283. Since we no longer need to apply styles based on :focus state, we can just set the width for the entire component which will ensure the append is included in that width.

+     className="kbnSearchBar"
      searchProps={{
        ...
-       className: 'kbnSearchBar',
      }}

And remove lines 17-21 in the Sass file because they were required solely for the animation.

-//TODO add these overrides to EUI so that search behaves the same globally (eui/issues/4363)
-.kbnSearchBar {
-  max-width: 100%;
-  will-change: width;
-}

cchaos · 2022-07-27T13:51:04Z

x-pack/plugins/global_search_bar/public/components/search_bar.tsx

        },
        fullWidth: true,
        append: showAppend ? (
          <EuiFormLabel
            title={keyboardShortcutTooltip}
-            css={{ fontFamily: euiTheme.font.familyCode }}
+            css={{ fontFamily: euiTheme.font.familyCode, position: 'absolute', right: 0 }}


Then you can delete this absolute positioning.

Suggested change

css={{ fontFamily: euiTheme.font.familyCode, position: 'absolute', right: 0 }}

css={{ fontFamily: euiTheme.font.familyCode }}

gsoldevila · 2022-07-27T16:14:15Z

Thanks a lot for the suggestions, much cleaner approach!

I see a small flickering issue caused by the append component being immediately black, whereas the rest of the search bar fades to black gradually, but I guess we can live with that:

Screen.Recording.2022-07-27.at.17.50.48.mov

x-pack/plugins/global_search_bar/public/components/search_bar.scss

…scss Co-authored-by: Caroline Horn <[email protected]>

kibana-ci · 2022-07-28T11:08:01Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`globalSearchBar`	21.1KB	20.5KB	-683.0B

History

💔 Build #61235 failed 1b8aa1f
💔 Build #61200 failed 8a7c688
💚 Build #61046 succeeded 9fb8a13
💚 Build #61041 succeeded 244f2b7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cchaos

Code & screen grabs LGTM, I didn't specifically pull down to test but quickly tried out a similar implementation in local EUI.

@cchaos

* Fix global search bar clear button glitch * Fix small layout shift caused by "append" disappearing on hover * Apply @cchaos 's suggestions * Update x-pack/plugins/global_search_bar/public/components/search_bar.scss Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Caroline Horn <[email protected]> (cherry picked from commit f2a14e2)

@cchaos

* Fix global search bar clear button glitch * Fix small layout shift caused by "append" disappearing on hover * Apply @cchaos 's suggestions * Update x-pack/plugins/global_search_bar/public/components/search_bar.scss Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Caroline Horn <[email protected]> (cherry picked from commit f2a14e2)

kibanamachine · 2022-07-28T16:31:56Z

💔 Some backports could not be created

Status	Branch	Result
❌	7.17	Backport failed because of merge conflicts
✅	8.3
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 137251

Questions ?

Please refer to the Backport tool documentation

@cchaos

* Fix global search bar clear button glitch * Fix small layout shift caused by "append" disappearing on hover * Apply @cchaos 's suggestions * Update x-pack/plugins/global_search_bar/public/components/search_bar.scss Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Caroline Horn <[email protected]> (cherry picked from commit f2a14e2) Co-authored-by: Gerard Soldevila <[email protected]>

@cchaos

* Fix global search bar clear button glitch * Fix small layout shift caused by "append" disappearing on hover * Apply @cchaos 's suggestions * Update x-pack/plugins/global_search_bar/public/components/search_bar.scss Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Caroline Horn <[email protected]> (cherry picked from commit f2a14e2) Co-authored-by: Gerard Soldevila <[email protected]>

Fix global search bar clear button glitch

244f2b7

gsoldevila requested review from a team as code owners July 27, 2022 08:02

gsoldevila added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:fix backport:all-open Backport to all branches that could still receive a release labels Jul 27, 2022

gsoldevila added v8.5.0 bug Fixes for quality problems that affect the customer experience labels Jul 27, 2022

Fix small layout shift caused by "append" disappearing on hover

9fb8a13

pgayvallet approved these changes Jul 27, 2022

View reviewed changes

cchaos reviewed Jul 27, 2022

View reviewed changes

gsoldevila changed the title ~~Fix global search bar clear button glitch~~ [Global search bar] Fix clear button glitch Jul 27, 2022

gsoldevila requested a review from cchaos July 27, 2022 13:30

cchaos suggested changes Jul 27, 2022

View reviewed changes

Apply @cchaos 's suggestions

8a7c688

gsoldevila requested a review from cchaos July 27, 2022 16:15

cchaos suggested changes Jul 27, 2022

View reviewed changes

x-pack/plugins/global_search_bar/public/components/search_bar.scss Outdated Show resolved Hide resolved

gsoldevila and others added 2 commits July 27, 2022 19:44

Update x-pack/plugins/global_search_bar/public/components/search_bar.…

1b8aa1f

…scss Co-authored-by: Caroline Horn <[email protected]>

Merge branch 'main' into kbn-135529-fix-search-bar-clear-button-glitch

ef5a001

gsoldevila requested a review from cchaos July 28, 2022 11:09

gsoldevila enabled auto-merge (squash) July 28, 2022 11:10

cchaos approved these changes Jul 28, 2022

View reviewed changes

gsoldevila merged commit f2a14e2 into elastic:main Jul 28, 2022

kibanamachine mentioned this pull request Jul 28, 2022

[8.3] [Global search bar] Fix clear button glitch (#137251) #137496

Merged

kibanamachine mentioned this pull request Jul 28, 2022

[8.4] [Global search bar] Fix clear button glitch (#137251) #137497

Merged

kibanamachine added the v8.3.3 label Jul 28, 2022

kibanamachine added the v8.4.0 label Jul 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Global search bar] Fix clear button glitch #137251

[Global search bar] Fix clear button glitch #137251

gsoldevila commented Jul 27, 2022 •

edited by kibanamachine

Loading

elasticmachine commented Jul 27, 2022

gsoldevila commented Jul 27, 2022

cchaos left a comment

cchaos Jul 27, 2022

gsoldevila Jul 27, 2022 •

edited

Loading

cchaos Jul 27, 2022

cchaos Jul 27, 2022

cchaos left a comment

cchaos Jul 27, 2022

gsoldevila commented Jul 27, 2022

kibana-ci commented Jul 28, 2022

cchaos left a comment

kibanamachine commented Jul 28, 2022

	css={{ fontFamily: euiTheme.font.familyCode, position: 'absolute', right: 0 }}
	css={{ fontFamily: euiTheme.font.familyCode }}

[Global search bar] Fix clear button glitch #137251

[Global search bar] Fix clear button glitch #137251

Conversation

gsoldevila commented Jul 27, 2022 • edited by kibanamachine Loading

elasticmachine commented Jul 27, 2022

gsoldevila commented Jul 27, 2022

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jul 27, 2022

Choose a reason for hiding this comment

gsoldevila Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

cchaos Jul 27, 2022

Choose a reason for hiding this comment

cchaos Jul 27, 2022

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jul 27, 2022

Choose a reason for hiding this comment

gsoldevila commented Jul 27, 2022

kibana-ci commented Jul 28, 2022

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 28, 2022

💔 Some backports could not be created

Manual backport

Questions ?

gsoldevila commented Jul 27, 2022 •

edited by kibanamachine

Loading

gsoldevila Jul 27, 2022 •

edited

Loading