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

Fix green outline at checkbox #13181

Merged
merged 6 commits into from
Dec 7, 2022
Merged

Fix green outline at checkbox #13181

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc065bb
fix blue outline at checkbox
hungvu193 Nov 30, 2022
00661af
fix edge case
hungvu193 Nov 30, 2022
01dd47c
Remove duplicate line, unused code
hungvu193 Nov 30, 2022
9ef959e
Merge remote-tracking branch 'origin/main' into fix-11283
hungvu193 Dec 2, 2022
da7be3b
update linting
hungvu193 Dec 2, 2022
fce96b1
update comment
hungvu193 Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/components/Checkbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ class Checkbox extends React.Component {
return;
}

const wasChecked = this.props.isChecked;

// If checkbox is checked and focused, make sure it's unfocused when pressed.
if (this.state.isFocused && wasChecked) {
this.onBlur();
}
Comment on lines +88 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a regression #18801. The onBlur should not be called explicitly, it should be called only when indeed the checkbox is blurred. This PR is trying to achieve focus-visible styles using focus state which won't work correctly due to the nature of how focus-visible style is maintained in browsers.

Copy link
Member

Choose a reason for hiding this comment

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

I think it was an intentional change. I am sure you will find a lot of discussion over it in the issue.


this.props.onPress();
}

Expand All @@ -94,6 +101,7 @@ class Checkbox extends React.Component {
onFocus={this.onFocus}
onBlur={this.onBlur}
ref={this.props.forwardedRef}
onPressOut={this.onBlur}
style={this.props.style}
onKeyDown={this.handleSpaceKey}
accessibilityRole="checkbox"
Expand All @@ -110,7 +118,7 @@ class Checkbox extends React.Component {
this.props.isChecked && styles.checkedContainer,
this.props.hasError && styles.borderColorDanger,
this.props.disabled && styles.cursorDisabled,
this.state.isFocused && styles.borderColorFocus,
(this.state.isFocused || this.props.isChecked) && styles.borderColorFocus,
]}
>
{this.props.isChecked && <Icon src={Expensicons.Checkmark} fill={themeColors.textLight} height={14} width={14} />}
Expand Down