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

[Controls] Make focus on the selected field when editing a control #162697

Closed
teresaalvarezsoler opened this issue Jul 28, 2023 · 4 comments · Fixed by #162067
Closed

[Controls] Make focus on the selected field when editing a control #162697

teresaalvarezsoler opened this issue Jul 28, 2023 · 4 comments · Fixed by #162067
Assignees
Labels
Feature:Dashboard Dashboard related features impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:Dashboard Usability Related to the Dashboard Usability initiative Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@teresaalvarezsoler
Copy link

Problem
When users edit an existing control and they have an index with hundreds of fields, it's very difficult to find which field is selected for the control.
image

Solution
When the user edits a control, make focus on the selected field.
image

@teresaalvarezsoler teresaalvarezsoler added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Project:Dashboard Usability Related to the Dashboard Usability initiative labels Jul 28, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@Heenawter Heenawter added loe:medium Medium Level of Effort and removed triage_needed labels Jul 28, 2023
@Heenawter
Copy link
Contributor

Heenawter commented Jul 28, 2023

We can't reliably set the focus to and/or scroll to the selected field - the options in the EuiSelectable component are virtualized, which means that (basically) only the visible options are rendered at a specific time. The benefit of this is that the list renders super fast, since it only has to render the HTML for ~10 options and not the entire list - however, it also means that the HTML for the selected field is not necessarily present on the screen yet, which means we can't force focus or scroll to it.

An alternative would be to bring the selected field to the top of the list (only on the first load - selecting a new field would not rearrange the list), much like we are doing for the navigation embeddable - would this be okay?

@teresaalvarezsoler
Copy link
Author

Thanks for the context @Heenawter

bring the selected field to the top of the list

This will work!

selecting a new field would not rearrange the list

Do you mean the previous selected field will remain at the top? That is ok. However, if I select a new field, close the editing flyout and open it again, the new field will appear on the top right? While the former selected field will appear in its natural position.

@Heenawter
Copy link
Contributor

Do you mean the previous selected field will remain at the top? That is ok. However, if I select a new field, close the editing flyout and open it again, the new field will appear on the top right? While the former selected field will appear in its natural position.

That is correct! :)

Heenawter added a commit that referenced this issue Aug 4, 2023
…rm (#162067)

Closes #162697

## Summary

This PR adds a few tiny UI improvements to the control creation
elements, including...
  
**Data view picker:**
- Made the `Data view` form title respond to focus as expected
  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
| ![Jul-28-2023
15-55-13](https://github.com/elastic/kibana/assets/8698078/c287978d-a54a-4809-a806-5a2caa41cf5d)
| ![Jul-28-2023
15-56-24](https://github.com/elastic/kibana/assets/8698078/8f403c2d-80a5-4fc1-989a-1ecceb056fc9)
|

- Switched to use `EuiInputPopover` rather than `EuiPopover`
- Removed the redundant popover title

  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
|
![image](https://github.com/elastic/kibana/assets/8698078/013fc848-3a9a-4280-9b37-6c1f025f3597)
| ![Screenshot 2023-07-28 at 4 16 18
PM](https://github.com/elastic/kibana/assets/8698078/22a2de30-cae1-49d4-9c33-d1537488d08d)
|

**Field picker:**
- Made the `Field` form row title respond to focus as expected for all
of the inner form elements
  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
| ![Jul-28-2023
16-06-01](https://github.com/elastic/kibana/assets/8698078/7dd845bc-0476-4b2a-b9b5-efce3c2e2844)
| ![Jul-28-2023
16-07-00](https://github.com/elastic/kibana/assets/8698078/222a9199-e5c2-4180-9501-e31588020855)
|

- Switched the `FieldTypeFilter` to use `EuiInputPopover` rather than
`EuiPopover`
- Removed the redundant title from the `FieldTypeFilter` popover
  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
|
![image](https://github.com/elastic/kibana/assets/8698078/007c61db-989b-4615-a36f-5f6307f04aaf)
|
![image](https://github.com/elastic/kibana/assets/8698078/ed7aea0c-d852-4f1c-ae03-14933fa2888a)
|

- Made changes described in
elastic/eui#6627 (comment) so
that, when the field type filter is closed (either via `Esc` or through
the natural tab order), the focus returns to the search field
  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
| ![Jul-28-2023
16-12-58](https://github.com/elastic/kibana/assets/8698078/aea49501-1f61-4ae8-bc90-1bacbbc232e7)
| ![Jul-28-2023
16-13-54](https://github.com/elastic/kibana/assets/8698078/8068b090-9cca-427f-bc36-2b9e6b2324f1)
|

- If provided, the initial selected field is now brought to the top of
the list

  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
|
![image](https://github.com/elastic/kibana/assets/8698078/2bdad643-d184-4c80-b940-5a73820dc8a5)
|
![image](https://github.com/elastic/kibana/assets/8698078/cda382e2-0e15-48c0-bdbf-c530a77570b8)
|

**Controls display settings:**

- Surrounded the `Minimum width` row with a `div` so that it can receive
the `id` passed down from the `EuiFormRow` and respond to focus as
expected

  | <div align="center">Before</div> | <div align="center">After</div> |
  |--------|--------|
| ![Jul-28-2023
16-31-56](https://github.com/elastic/kibana/assets/8698078/125d2a75-bcec-452c-8682-85de3a44185b)
| ![Jul-28-2023
16-31-20](https://github.com/elastic/kibana/assets/8698078/935a17f1-4adc-4b86-811b-334a42e4627e)
|


### Checklist

- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Jatin Kathuria <jatin.kathuria@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:Dashboard Usability Related to the Dashboard Usability initiative Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants