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

[Dashboard/Markdown] Add tabindex #197848

Conversation

kowalczyk-krzysztof
Copy link
Member

@kowalczyk-krzysztof kowalczyk-krzysztof commented Oct 25, 2024

Summary

This PR adds tabIndex to markdown_vis_controller.tsx.

Closes: #186559

Visuals:

chrome_aWPkAz51AM.mp4

This video demonstrates the markdown panel being tabbable and the scrollbar in being controlled with keyboard.

@kowalczyk-krzysztof kowalczyk-krzysztof added bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Markdown Markdown visualization feature release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 25, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof self-assigned this Oct 25, 2024
@kibanamachine kibanamachine added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Oct 25, 2024
@elasticmachine
Copy link
Contributor

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

2 similar comments
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@kowalczyk-krzysztof kowalczyk-krzysztof added loe:small Small Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Oct 25, 2024
@Heenawter Heenawter self-requested a review October 25, 2024 15:10
@kowalczyk-krzysztof
Copy link
Member Author

Looks like the shared-ux markdown component behaves differently when it comes to HTML. I'm converting this back to draft as I investigate what is happening.

@kowalczyk-krzysztof kowalczyk-krzysztof marked this pull request as draft October 28, 2024 09:10
@kowalczyk-krzysztof kowalczyk-krzysztof changed the title [Dashboard/Markdown] Add tabindex, replace legacy markdown [Dashboard/Markdown] Add tabindex Oct 29, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof marked this pull request as ready for review October 29, 2024 08:48
@kowalczyk-krzysztof
Copy link
Member Author

kowalczyk-krzysztof commented Oct 29, 2024

Looks like the shared-ux markdown component behaves differently when it comes to HTML. I'm converting this back to draft as I investigate what is happening.

So markdown-vis-controller uses markdown-it library under the hood while the shared-ux component uses EuiMarkdownEditor and EuiMarkdownFormat (depending on readOnly flag) and figuring out differences between the underlaying markdown rendering engines is way out of scope of this. So I changed the PR to only include the tabIndex change.

EDIT: Link to an issue about standardizing markdown across kibana, which I didn't know existed before I tried replacing markdown in this PR.

Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding tabIndex LGTM - quick test locally to ensure keyboard interactivity + code review 👍

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeMarkdown 5.8KB 5.8KB +11.0B

History

  • 💔 Build #246009 failed 81e3b51dccca87fa41a8736410d397105e839e23

cc @kowalczyk-krzysztof

@kowalczyk-krzysztof kowalczyk-krzysztof merged commit 6cc8b97 into elastic:main Oct 30, 2024
22 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11596906820

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 30, 2024
## Summary

This PR adds `tabIndex` to
[markdown_vis_controller.tsx](https://github.com/elastic/kibana/compare/main...kowalczyk-krzysztof:kibana:fix/dashboard-markdown-panel?expand=1#diff-47267cf2f7d8f9e72e157ddb40292226a88ad5e51f9c92486799d381c5085e5f).

Closes: elastic#186559

## Visuals:

https://github.com/user-attachments/assets/8264c584-e7ca-4647-946a-7b9bd9f7aa8f

This video demonstrates the markdown panel being tabbable and the
scrollbar in being controlled with keyboard.

(cherry picked from commit 6cc8b97)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 30, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Dashboard/Markdown] Add tabindex
(#197848)](#197848)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Krzysztof
Kowalczyk","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-30T15:53:19Z","message":"[Dashboard/Markdown]
Add tabindex (#197848)\n\n## Summary\r\n\r\nThis PR adds `tabIndex`
to\r\n[markdown_vis_controller.tsx](https://github.com/elastic/kibana/compare/main...kowalczyk-krzysztof:kibana:fix/dashboard-markdown-panel?expand=1#diff-47267cf2f7d8f9e72e157ddb40292226a88ad5e51f9c92486799d381c5085e5f).\r\n\r\nCloses:
#186559\r\n\r\n##
Visuals:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8264c584-e7ca-4647-946a-7b9bd9f7aa8f\r\n\r\nThis
video demonstrates the markdown panel being tabbable and
the\r\nscrollbar in being controlled with
keyboard.","sha":"6cc8b97b60e78bf0638fa91b0925eb944133841a","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Dashboard","Feature:Markdown","Team:Presentation","loe:small","release_note:skip","impact:medium","v9.0.0","backport:prev-minor"],"title":"[Dashboard/Markdown]
Add
tabindex","number":197848,"url":"https://github.com/elastic/kibana/pull/197848","mergeCommit":{"message":"[Dashboard/Markdown]
Add tabindex (#197848)\n\n## Summary\r\n\r\nThis PR adds `tabIndex`
to\r\n[markdown_vis_controller.tsx](https://github.com/elastic/kibana/compare/main...kowalczyk-krzysztof:kibana:fix/dashboard-markdown-panel?expand=1#diff-47267cf2f7d8f9e72e157ddb40292226a88ad5e51f9c92486799d381c5085e5f).\r\n\r\nCloses:
#186559\r\n\r\n##
Visuals:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8264c584-e7ca-4647-946a-7b9bd9f7aa8f\r\n\r\nThis
video demonstrates the markdown panel being tabbable and
the\r\nscrollbar in being controlled with
keyboard.","sha":"6cc8b97b60e78bf0638fa91b0925eb944133841a"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197848","number":197848,"mergeCommit":{"message":"[Dashboard/Markdown]
Add tabindex (#197848)\n\n## Summary\r\n\r\nThis PR adds `tabIndex`
to\r\n[markdown_vis_controller.tsx](https://github.com/elastic/kibana/compare/main...kowalczyk-krzysztof:kibana:fix/dashboard-markdown-panel?expand=1#diff-47267cf2f7d8f9e72e157ddb40292226a88ad5e51f9c92486799d381c5085e5f).\r\n\r\nCloses:
#186559\r\n\r\n##
Visuals:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8264c584-e7ca-4647-946a-7b9bd9f7aa8f\r\n\r\nThis
video demonstrates the markdown panel being tabbable and
the\r\nscrollbar in being controlled with
keyboard.","sha":"6cc8b97b60e78bf0638fa91b0925eb944133841a"}}]}]
BACKPORT-->

Co-authored-by: Krzysztof Kowalczyk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Markdown Markdown visualization feature impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility - Cannot fully use keyboard to select and scroll on a Dashboard Text/Markdown panel
4 participants