Skip to content
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

[Global header] Review feedback and suggestions for small visual improvements #77663

Closed
7 tasks done
formgeist opened this issue Sep 16, 2020 · 10 comments
Closed
7 tasks done
Labels
Feature:Navigational Search Global search bar Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.0.0

Comments

@formgeist
Copy link
Contributor

formgeist commented Sep 16, 2020

Hi team 👋,

First off - amazing that we have a global search experience in Kibana 🙌 Simply amazing!

I've been testing out the new global header and search and I've found some minor visual nits and possibly things that have already come up and will be fixed, but I thought to collect it anyways. Feel free to let me know if you want to separate any of these out, I first and foremost just wanted to deliver this feedback.


  • Text label when hovering the options shows undefinedundefined

Screenshot 2020-09-14 at 21 56 40

  • The search area is quite small and the clear option covers up the text inside when typing a lot of text

Screenshot 2020-09-14 at 22 00 53

  • The position of the popover is just a pixel above the secondary header bar. Either we should make the separation bigger, it just looks like it's off but not enough at the moment ([GS] Fix placement of the popover #147708)

Screenshot 2020-09-14 at 22 01 27

  • The Kibana loading bar sits at the very top, but because the dark background and accent colored loader, it's not very visible. There are a few places where one would not notice the loader and simply think the page is done. I would propose putting the loader under the global header or the secondary menu bar so it can be seen and it's closer to the content below it.

Kapture 2020-09-14 at 23 09 44

  • If the datepicker is open and you open the search popover, both close (Chrome)

Kapture 2020-09-16 at 20 50 35

  • Naming our "Overview" pages the same is nice for consistency, but because we're only showing an icon, it's hard to know which overview page I'm trying to locate. I feel like this was discussed as some point.

Screenshot 2020-09-16 at 20 52 38

Kapture 2020-09-16 at 20 56 17

@formgeist formgeist added v8.0.0 REASSIGN from Team:Core UI Deprecated label for old Core UI team Feature:Header Work related to the header section of the Kibana app. labels Sep 16, 2020
@elasticmachine
Copy link
Contributor

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

@myasonik
Copy link
Contributor

Text label when hovering the options shows undefinedundefined

Fixed in elastic/eui#4008. Will be merged into Kibana with EUIv29.0.0

Naming our "Overview" pages the same is nice for consistency, but because we're only showing an icon, it's hard to know which overview page I'm trying to locate. I feel like this was discussed as some point.

Fixed in #77662 (tracked in #77114)

@cchaos
Copy link
Contributor

cchaos commented Sep 16, 2020

Nice @formgeist ! You've definitely found a few we're aware of and a few we're not. I'll reply to what I can attest to from the EUI side.


The search area is quite small and the clear option covers up the text inside when typing a lot of text

@myasonik Are you somehow removing the isClearable functionality? The euiFieldSearch-isClearable class seems to be missing from the input's class list.

This is the screenshot from EUI docs:
Screen Shot 2020-09-16 at 15 24 21 PM


The position of the popover is just a pixel above the secondary header bar. Either we should make the separation bigger, it just looks like it's off but not enough at the moment

Ah the devil that is pixel perfection. Unfortunately this is just how all popovers render in terms of distance from it's triggering element and how it happens to line up with the header. I think it's a tiny detail that'll only bug those eagle-eye designers.


If the datepicker is open and you open the search popover, both close (Chrome)

This is an artifact of bad focus behavior of EuiPopover in general and we're actively working on a fix in EUI elastic/eui#4003


Hitting CMD will force to scroll to the top of list. I did this when I wanted to switch to another tab, and found the experience a little jarring. Not sure if this was something to do with the shortcut or a11y?

Hmmm, this is going to be a difficult one, and probably has to be solved in EuiSelectable. What's happening is that the "focus" is on the first item on the list. As soon as you scroll and then hit any kind of key, it wants to scroll the focused item back into view because without mouse interaction it assumes you'll be pressing another key to act on the focused item.

@myasonik May need a bit of help there on the EUI JS side.

@myasonik
Copy link
Contributor

myasonik commented Sep 16, 2020

@myasonik Are you somehow removing the isClearable functionality? The euiFieldSearch-isClearable class seems to be missing from the input's class list.

Something seems to have gone weird with EUI deployments. It was fixed in 28.1.0 but is missing from 28.2.0...

It's back in master so I'm not super concerned... but weird.

Anyway, it should be fixed next time EUI is updated in Kibana.

Edit: Confirmed with Chandler that the fix was merged into 28.3.1 and that the changelog edited incorrectly. Kibana's on 28.2.0 right now.


Hmmm, this is going to be a difficult one, and probably has to be solved in EuiSelectable. What's happening is that the "focus" is on the first item on the list. As soon as you scroll and then hit any kind of key, it wants to scroll the focused item back into view because without mouse interaction it assumes you'll be pressing another key to act on the focused item.

I'm not really sure if we can tackle this one right now. If I understand the original flow correctly, @formgeist, you were going to try to CMD+click on an option to open it in a new tab? If so, the options aren't links so this doesn't actually work anways. We do need to explore whether or not we can make these options act like links as well but for now they don't because by default this would break screen reader expectations of the semantics of a listbox (the options list). Because this would be exploratory work, it's hard to estimate how much effort this is.

All that said, that's assuming you were going to CMD+click on an option. If not, I'd love to hear what you were trying to accomplish when pressing CMD.

@formgeist
Copy link
Contributor Author

All that said, that's assuming you were going to CMD+click on an option. If not, I'd love to hear what you were trying to accomplish when pressing CMD.

That was the main reason to using CMD+click - to open a given link in a new tab in case I wanted to open e.g. several saved objects, or APM services. I imagine this becomes more apparent when we start seeing more result types in the search suggestions.

@afharo afharo added Feature:Navigational Search Global search bar and removed Feature:Header Work related to the header section of the Kibana app. labels Dec 16, 2022
@botelastic botelastic bot added the needs-team Issues missing a team label label Dec 16, 2022
@afharo afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc and removed REASSIGN from Team:Core UI Deprecated label for old Core UI team labels Dec 16, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Dec 16, 2022
@afharo
Copy link
Member

afharo commented Dec 16, 2022

The search area is quite small and the clear option covers up the text inside when typing a lot of text

I think this is already solved as well:
image

Let me know if you think otherwise.


The Kibana loading bar sits at the very top, but because the dark background and accent colored loader, it's not very visible. There are a few places where one would not notice the loader and simply think the page is done. I would propose putting the loader under the global header or the secondary menu bar so it can be seen and it's closer to the content below it.

The loading bar was replaced by the spinner, which has better contrast. I'd say we also ticked this part :)


If the datepicker is open and you open the search popover, both close (Chrome)

I just tested it on Chrome and Firefox and it also looks like it's been resolved.


Hitting CMD will force to scroll to the top of list. I did this when I wanted to switch to another tab, and found the experience a little jarring. Not sure if this was something to do with the shortcut or a11y?

It looks like there's some discussion around this in elastic/eui#5408. Although scrolling to the top doesn't happen anymore (it navigates to the app), it doesn't open the link in a new tab.


The position of the popover is just a pixel above the secondary header bar. Either we should make the separation bigger, it just looks like it's off but not enough at the moment

We also need to fix this... I just tested it and the placement is still the same.

@formgeist
Copy link
Contributor Author

@afharo Thanks for looking into this 👍 It does appear that most of my concerns have been addressed since this issue was created. I did find a small bug when navigating the search popover #147809 Just thought I'd mention that in here as well

@TinaHeiligers TinaHeiligers added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Feb 14, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@TinaHeiligers TinaHeiligers removed the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 14, 2023
@rshen91
Copy link
Contributor

rshen91 commented May 14, 2024

@elastic/kibana-design can you confirm for me that there are no remaining issues in this? I don't see any but want to confirm, thank you

@formgeist
Copy link
Contributor Author

@rshen91 I've just checked and it appears the last item in the original list has been fixed in the meantime

Hitting CMD will force to scroll to the top of list. I did this when I wanted to switch to another tab, and found the experience a little jarring. Not sure if this was something to do with the shortcut or a11y?

We can close this initial design feedback issue and if anything else pops up we can address it in a new issue 👍 Happy to close this after almost 3,5 years 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Navigational Search Global search bar Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.0.0
Projects
None yet
Development

No branches or pull requests

7 participants