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.

Sign up for GitHub

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

Jump to bottom

Remove tabIndex from simplebar scroll containers #30403

Closed
shubham-padia opened this issue Jun 12, 2024 · 1 comment · Fixed by #30444
Closed

Remove tabIndex from simplebar scroll containers #30403

shubham-padia opened this issue Jun 12, 2024 · 1 comment · Fixed by #30444
Assignees
@shubham-padia
Labels
area: left sidebar (ui) Left sidebar buttons/interactions excluding popovers bug help wanted in progress

Comments

@shubham-padia
Copy link
Member

While working on Add filter to direct messages list. #30332, I was addressing the feedback that the tab behaviour was weird when going from the search box to a conversation row. This was because the focus was moving on to the simplebar scroll container but it had no visual feedback. Right now, I'm setting the tabindex to -1 in my particular along with some other places in the code doing so too.

In the current CZO deployment also, if you start tabbing in the left sidebar from the top, after the direct messages header, you'll notice that you'll have to press an extra tab to go from the direct messages header to the first conversation row.

CZO thread
@shubham-padia shubham-padia self-assigned this Jun 12, 2024
@shubham-padia shubham-padia mentioned this issue Jun 12, 2024
Open
12 tasks
@alya alya added area: left sidebar (ui) Left sidebar buttons/interactions excluding popovers help wanted labels Jun 12, 2024
@shubham-padia
Copy link
Member Author

Opened Grsmto/simplebar#695, hopefully that'll be merged and we won't have to think of a workaround

shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 16, 2024
@shubham-padia
scroll: Set tabIndex to -1 for simplebar content wrapper.
6c4d556 
Fixes zulip#30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore becuase the wrapper is
not focusable anymore.
@shubham-padia shubham-padia mentioned this issue Jun 16, 2024
Merged
12 tasks
shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 18, 2024
@shubham-padia
scroll: Set tabIndex to -1 for simplebar content wrapper.
35c5b20 
Fixes zulip#30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore becuase the wrapper is
not focusable anymore.
shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 19, 2024
@shubham-padia
scroll: Set tabIndex to -1 for simplebar content wrapper.
042bb8d 
Fixes zulip#30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore becuase the wrapper is
not focusable anymore.
shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 19, 2024
@shubham-padia
scroll: Set tabIndex to -1 for simplebar content wrapper.
bb4c929 
Fixes zulip#30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore becuase the wrapper is
not focusable anymore.
shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 19, 2024
@shubham-padia
scroll: Set tabIndex to -1 for simplebar content wrapper.
fe11f3b 
Fixes zulip#30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore because the wrapper is
not focusable anymore.
timabbott pushed a commit that referenced this issue Jun 20, 2024
@shubham-padia @timabbott
scroll: Set tabIndex to -1 for simplebar content wrapper.
43eebbf 
Fixes #30403.
Having tabIndex set to 0 led to keyboard focus being put on
a scrollbar container, which led to users having to tab twice
to skip a container.
This commit also removes instances of tabIndex being set to
-1 programatically for certain cases, since it is -1 by default now.
This commit also removes `outline: none` for simplebar since
that property is not needed anymore because the wrapper is
not focusable anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shubham-padia shubham-padia

Labels
area: left sidebar (ui) Left sidebar buttons/interactions excluding popovers bug help wanted in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

scroll: Set tabIndex to -1 for simplebar content wrapper. shubham-padia/zulip
3 participants
@alya @shubham-padia @zulipbot