Closed lanitochka17 closed 10 months ago
Triggered auto assignment to @isabelastisser (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Job added to Upwork: https://www.upwork.com/jobs/~01e6211747d5d8420e
Platforms
in OP are β
)Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws (External
)
Blue border appears around the row and the field is not focused when using Tab key. This issue happens to all TextInput
components.
The container wrapping text input is intentionally set with tabIndex=-1
to avoid navigating by tab to focus on itself.
However inside WebGenericPressable
, we do not pass tabIndex
properly:
Previously we used focusable
to determine tabIndex
. However, now focusable
is replaced with tabIndex
due to RNW upgrade. Therefore, if tabIndex
is used, we should prioritize it and should not depend merely on focusable
(deprecated) and accessible
.
Pass tabIndex
like this:
tabIndex={props.tabIndex ?? (!accessible || !focusable ? -1 : 0)}
NA
Blue border appears on the row when pressing tab key in the address page
To prevent focusing by tab, we set tabIndex=-1
below
https://github.com/Expensify/App/blob/3939beaf7969fb04bc46083b395a45118a406b05/src/components/TextInput/BaseTextInput/index.js#L258
but tabIndex
is changed here
https://github.com/Expensify/App/blob/3939beaf7969fb04bc46083b395a45118a406b05/src/components/Pressable/GenericPressable/index.tsx#L16
This is the root cause
I think we need to use props.tabIndex
if exists
Update
https://github.com/Expensify/App/blob/3939beaf7969fb04bc46083b395a45118a406b05/src/components/Pressable/GenericPressable/index.tsx#L16
to
tabIndex={props.tabIndex ?? (!accessible || !focusable) ? -1 : 0}
This works as expected
Hey @Ollyws, can you please review the proposals above? Thanks!
@Ollyws please provide an update. Thanks!
This is a regression caused by #31118, the fix for #31098.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
tabIndex
prop when it is defined from parent component.0
as tabIndex. And thus we can't use short hand operator ||
like this tabIndex={props.tabIndex || (!accessible || !focusable ? -1 : 0)}
as it will not forward 0
as tabIndex.const tabIndex = props.tabIndex !== undefined ? props.tabIndex : !accessible || !focusable ? -1 : 0;
tabIndex={tabIndex}
BaseTextInput/index.js
then we pass accessible={false}
like we did in CheckboxWithLabel
.
This also resolves our issue but i wasn't sure if this could affect other parts of the code or not.
@Ollyws, @isabelastisser Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@Ollyws, @isabelastisser Huh... This is 4 days overdue. Who can take care of this?
Triggered auto assignment to @deetergp, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
π£ @Ollyws π An offer has been automatically sent to your Upwork account for the Reviewer role π Thanks for contributing to the Expensify app!
π£ @tienifr π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
PR ready for review https://github.com/Expensify/App/pull/31810.
@Ollyws any updates here?
PR is merged.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.5-7 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-12-07. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
@Ollyws can you please complete the list above? Thanks!
@isabelastisser I accepted, thanks!
Bump @Ollyws to complete the checklist above. Thanks!
Hey @Ollyws, can you please complete the checklist above? Thanks!
BugZero Checklist:
- [x] The PR that introduced the bug has been identified. Link to the PR:
This started occuring due to the RNW upgrade but we can't really call it a regression as it's out of our hands.
- [x] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
N/A
- [x] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
N/A
- [x] Determine if we should create a regression test for this bug.
This is a minor visual issue that doesn't really affect the flow in any significant way so I don't think a regression test is necessary here.
Thanks! @Ollyws, please accept the offer in Upwork.
Accepted, thanks!
@isabelastisser The job is still open and unpaid on upwork for me if you could take a look, thanks!
All set!
If you havenβt already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.3.98-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
Action Performed:
Expected Result:
The next field will be focused with just one click
Actual Result:
A blue border appears around the field and the next field is not focused. It is only focused after pressing Tab key for the second time This issue also happens in other area of the app that involve entering value
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/85a2b1e3-7aa6-460e-a468-b398c566ccbc
View all open jobs on GitHub
Upwork Automation - Do Not Edit