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

Mute default :focus form highlight #36103

Merged
merged 3 commits into from
Oct 18, 2024
Merged

Mute default :focus form highlight #36103

merged 3 commits into from
Oct 18, 2024

Conversation

josuke0227
Copy link
Contributor

The applied border colour (border-color: #000;) is cascaded by the default border highlight (blue colour in Chrome). Therefore I reckon the default styling needs to be muted.

The applied border colour (border-color: #000;) is cascaded by the default border highlight (blue colour in Chrome). Therefore I reckon the default styling needs to be muted.
@josuke0227 josuke0227 requested a review from a team as a code owner September 29, 2024 06:42
@josuke0227 josuke0227 requested review from hamishwillee and removed request for a team September 29, 2024 06:42
@github-actions github-actions bot added Content:Learn:Forms Learning area Forms docs size/xs [PR only] 0-5 LoC changed labels Sep 29, 2024
Copy link
Contributor

github-actions bot commented Sep 30, 2024

Preview URLs

(comment last updated: 2024-10-18 02:20:18)

@hamishwillee
Copy link
Collaborator

hamishwillee commented Sep 30, 2024

Thanks - I am closing this because I think it is invalid. There don't appear to be any outline properties set on the box either before or after this change, and as far as I can tell there isn't any visual change either.

If you can show exactly what the before and after change is (screenshots) then perhaps we can discuss. Note that any changes made here also have to be made in the corresponding example code linked from the page.

@josuke0227
Copy link
Contributor Author

josuke0227 commented Sep 30, 2024

Thanks for your response.

Here's where I reproduced the change:
https://developer.mozilla.org/en-US/docs/Learn/Forms/Your_first_form#summary

Here's how I applied the change and confirmed both the property is applied to the element and the appearance is affected as per the property.

Reproduction process 1
Reporoduction_process1

Reproduction process 2
Reproduction_result2

Result
Result

@hamishwillee
Copy link
Collaborator

hamishwillee commented Sep 30, 2024

Thank you, but I cannot reproduce. Using the current version you can see below the border is black to start with and that there is no computed property for outline - i.e. no blue outline that is inherited. It may be that this is set differently on another browser, but I still think adding this fix would be more confusing than helpful.

image

@josuke0227
Copy link
Contributor Author

josuke0227 commented Sep 30, 2024

Hi, I tried to see how the focused form's appearance changes using my Windows laptop and I've got the same outline as yours so the difference in border colour can be concluded in the difference of the OS.

I also found the style (either blue or black) is defined in ":forcus-visible" pseudo selector.
Here is the evidence:

Process1

Your "outline-color" might be "rgb(16, 16, 16)"
Result

From above, if you need to change the border colour to #000, you need the "outline: none" to make the border colour visible. However, it makes sense that it will be confusing as you mentioned. What about styling like below by using "outline-color" instead of "border-color"?

input:focus,
textarea:focus {
/* To give a little highlight on active elements */
outline-color: #000;
}

@hamishwillee
Copy link
Collaborator

THanks for your patience @josuke0227 .

Yes, I think that instead of muting the outline, what you have suggested will work better, and more important, be more consistent across browsers.

input:focus,
textarea:focus {
/* To give a little highlight on active elements */
outline-color: #000;
}

BUT, as I noted before, we also need to make this same change in https://github.com/mdn/learning-area/blob/main/html/forms/your-first-HTML-form/first-form-styled.html and the other linked examples - can you do that too, and link to here?

This should be tested on at least chrome and FF.

@hamishwillee hamishwillee reopened this Oct 1, 2024
@josuke0227
Copy link
Contributor Author

josuke0227 commented Oct 1, 2024

Hi,
I have just tested the "outline-color" with Chrome and FF. It seems that "outline-color" does not work without "outline-style" property on FF.

My suggestion is to use the following style instead of the past one.
This styling is tested and confirmed that it works for both Chrome and FF.
Please see the attached images below.

input:focus,
textarea:focus {
/* Set the outline width and style /
outline-style: solid;
/
To give a little highlight on active elements */
outline-color: #000;
}

  • Evidence of not working
evidence-not-working-ff
  • Evidence of working (Chrome)
evidence-working-chrome
  • Evidence of working (FF)
evidence-working-ff

Regarding the below comment, thank you for sharing the URL.
As far as I checked, there do not seem to be similar cases except the URL you shared but I will open pull requests with the link to here as soon as I find them.

BUT, as I noted before, we also need to make this same change in https://github.com/mdn/learning-area/blob/main/html/forms/your-first-HTML-form/first-form-styled.html and the other linked examples - can you do that too, and link to here?

@hamishwillee
Copy link
Collaborator

@josuke0227 - thank you for the testing and confirmation.

Can you apply your fix here, and cross link from your PR to https://github.com/mdn/learning-area/blob/main/html/forms/your-first-HTML-form/first-form-styled.html

I can then review and merge them together.

@josuke0227
Copy link
Contributor Author

Hi @hamishwillee,
Thanks for checking the results.

I've just updated the other one.
Please check out the link below.
mdn/learning-area#767

Thank you!

@hamishwillee
Copy link
Collaborator

Thanks @josuke0227 - might not get to this until Friday, but have not forgotten!

@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/xs [PR only] 0-5 LoC changed labels Oct 18, 2024
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks very much @josuke0227 - also for your patience with my review.

@hamishwillee hamishwillee merged commit f575c31 into mdn:main Oct 18, 2024
8 checks passed
@josuke0227 josuke0227 deleted the patch-2 branch October 18, 2024 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Learn:Forms Learning area Forms docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants