[JENKINS-69276] No focus state on signin page checkbox

[frankie139506]

See JENKINS-69276.

Proposed changelog entries

• Add focus state on sign in page checkbox.

Proposed upgrade guidelines

N/A

Submitter checklist

• [X] (If applicable) Jira issue is well described
• [X] Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• [X] Appropriate autotests or explanation to why this change has no tests
• [ ] New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
• [ ] New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO") if applicable.
• [ ] New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content-Security-Policy directives (see documentation on jenkins.io).
• [ ] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

• [ ] There are at least 2 approvals for the pull request and no outstanding requests for change
• [ ] Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• [ ] Changelog entries in the PR title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood
• [ ] Proper changelog labels are set so that the changelog can be generated automatically
• [ ] If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• [ ] If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[timja]

Do you know what caused it to lose this and is this undo-ing / affecting another change?

[frankie139506]

Do you know what caused it to lose this and is this undo-ing / affecting another change?

I found it relate to JENKINS-68788, I think this PR only need to remove the canceled focus state, but In JENKINS-68788 it remove all the focus state this is very odd.

[timja]

Tim, do I understand your issue correctly that there's no post-click focus state on the checkbox, but you want one here? That's what the PR adds, but this is not what we do for checkboxes in general.

No, the issue is when you tab across it its invisible you do not know it's selected

[NotMyFault]

No, the issue is when you tab across it its invisible you do not know it's selected

Oic, thanks for the clarification. My former question turns into an issue then.

[frankie139506]

This doesn't seem to be working for me, your video show clicking but the issue is about when tabbing through elements.

i.e. click username, see focus state:

Then press tab twice and you can't tell the checkbox is selected.

(apologies if the issue wasn't clear enough)

Thanks for explaining. Don't we need a focus state when click the checkbox?

[timja]

Thanks for explaining. Don't we need a focus state when click the checkbox?

Yes there should be some, looking closer I see you've fixed one half of this.

If it's selected it now works properly, i.e. if you click keep me signed in and then tab off it and back to it it's correctly got a focus state.

but the unchecked state is not working.

(so currently this is better than currently released version but would be good to fix the default state)

[frankie139506]

Thanks for explaining. Don't we need a focus state when click the checkbox?

Yes there should be some, looking closer I see you've fixed one half of this.

If it's selected it now works properly, i.e. if you click keep me signed in and then tab off it and back to it it's correctly got a focus state.

but the unchecked state is not working.

(so currently this is better than currently released version but would be good to fix the default state)

I have made some change, please take a look.

[NotMyFault]

No, the issue is when you tab across it its invisible you do not know it's selected

Oic, thanks for the clarification. My former question turns into an issue then.

Keep in mind that my issue outlined previously remained unaddressed and introduces a future regression, if that is a considerable merge blocker:

https://user-images.githubusercontent.com/13383509/184725774-0be7b5d4-5765-45a5-9119-58ca43d98cb8.mp4

[frankie139506]

outlined

@NotMyFault It seems to me that if I click the checkbox, we get the focus state, that's why when you uncheck the checkbox, they still have the focus state. If I remove the focus state when unchecked, it will be asymmetrical, which is my humble opinion.

[timja]

@NotMyFault I see no issue there, when you uncheck it your focus state is on the checkbox. it should be highlighted

[NotMyFault]

@NotMyFault I see no issue there, when you uncheck it your focus state is on the checkbox. it should be highlighted

If this is intended, it's incosistent with the overall checkbox design, see https://weekly.ci.jenkins.io/design-library/Checkboxes/ for example.

[timja]

@NotMyFault I see no issue there, when you uncheck it your focus state is on the checkbox. it should be highlighted

If this is intended, it's incosistent with the overall checkbox design, see weekly.ci.jenkins.io/design-library/Checkboxes for example.

It's not inconsistent, it's a bug this PR is fixing. Which is why I updated the title and the changelog entry.

Try tabbing across the job configure page its impossible right now to know where you are -.-

[NotMyFault]

Which is why I updated the title and the changelog entry.

Oic, I didn't notice this one, let's disregard my concern 👍🏻