fix: Dialog Component: can't scroll with keyboard in 'Long content' example.#35612
fix: Dialog Component: can't scroll with keyboard in 'Long content' example.#35612krasnoff wants to merge 13 commits intomicrosoft:masterfrom
Conversation
|
@microsoft-github-policy-service agree |
| // `ref` is wrongly assigned to be `HTMLElement` instead of `HTMLDivElement` | ||
| // but since it would be a breaking change to fix it, we are casting ref to it's proper type | ||
| ref: ref as React.Ref<HTMLDivElement>, | ||
| tabIndex: 0, |
There was a problem hiding this comment.
pls follow https://storybooks.fluentui.dev/react/?path=/docs/concepts-developer-accessibility-focus-indicator--docs to add focus indicator.
in general - I am not sure if this is applicable in all cases.
It is certainly not needed for smaller dialogs.
For larger ones there can be a focusable element in the content which will allow users to scroll when after move focus.
Maybe an optional prop? @kolaps33 @smhigley @bsunderhus what do you think?
There was a problem hiding this comment.
I added tabIndex: 0 in order to make the component focusable and thus to enable scrolling.
I dont quite understand why do we have to add focus indicator.
There was a problem hiding this comment.
attach screenshot that proves that the component can be focusable without any neccessary addons.
There was a problem hiding this comment.
I changed the element to HTMLDivElement
…CSS (microsoft#35716) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Hotell <1223799+Hotell@users.noreply.github.com>
…ove unused base hooks (microsoft#35689) Co-authored-by: Martin Hochel <hochelmartin@gmail.com>
…crosoft#35720) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dmytrokirpa <1492102+dmytrokirpa@users.noreply.github.com> Co-authored-by: Dmytro Kirpa <dmytrokirpa@microsoft.com>
krasnoff
requested review from
a team,
JustSlone,
Mitch-At-Work and
tudorpopams
as code owners
| @@ -494,8 +494,8 @@ export const styles = css` | |||
| right: 0; | |||
| border-radius: inherit; | |||
| transition-property: margin, opacity; | |||
| transition-duration: ${durationUltraSlow}), ${durationSlower}; | |||
| transition-delay: ${animations.fastEase}), ${animations.nullEasing}); | |||
| transition-duration: ${durationUltraSlow}, ${durationSlower}; | |||
| transition-delay: ${animations.fastEase}, ${animations.nullEasing}; | |||
There was a problem hiding this comment.
Are these being re-committed in this merge commit?
Previous Behavior
In the Dialog Scrolling Long Content example, the dialog content is not scrollable with keyboard.
New Behavior
Now the dialog conent is scrollable by keyboard when it is in focus.
Related Issue(s)
#35603