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

Add <label> to provider name #387

Merged
merged 4 commits into from
Oct 7, 2022
Merged

Add <label> to provider name #387

merged 4 commits into from
Oct 7, 2022

Conversation

r-a-y
Copy link
Contributor

@r-a-y r-a-y commented Oct 3, 2020

Hi,

This PR wraps the provider name with the <label> element for better UX.

GIF:

GIF

I've also renamed the Name column to Type since this column also has buttons depending on the provider (TOTP, Backup Verification Codes). I can revert this if this isn't desired.

@jeffpaul jeffpaul added this to the 0.8.0 milestone Apr 9, 2021
@jeffpaul
Copy link
Member

@georgestephanis / @kasparsd any thoughts on a review of this PR and whether it's ready for consideration in v0.8.0?

@jeffpaul
Copy link
Member

Given that we're looking at a larger UX review of this part of the plugin and that in yesterday's bug scrub @georgestephanis is not certain the label targeting the tickbox instead of the radio button when the radio button is closer and that it might be possible to flip those columns as well; I'm going to punt this to a future release to slim down our focus for 0.8.0 to handle the larger U2F deprecation.

@jeffpaul jeffpaul modified the milestones: 0.8.0, Future Release Mar 24, 2022
@r-a-y
Copy link
Contributor Author

r-a-y commented Mar 24, 2022 •
edited
Loading

Given that we're looking at a larger UX review of this part of the plugin

Yeah, the Enabled / Primary UX needs to be overhauled (see #342). I'm injecting the labels with some javascript at the moment as a workaround until the interface is reworked at some point.

@kasparsd
Copy link
Collaborator

kasparsd commented Oct 7, 2022

This is a great UX and accessibility improvement. Let's merge this in.

@kasparsd kasparsd merged commit eb55071 into WordPress:master Oct 7, 2022
@kasparsd kasparsd mentioned this pull request Oct 7, 2022
Merged
@jeffpaul jeffpaul modified the milestones: Future Release, 0.8.0 Oct 7, 2022
@r-a-y r-a-y deleted the try/input-checkbox-labels branch October 7, 2022 20:34
@kasparsd kasparsd mentioned this pull request Oct 17, 2022
Merged
@jeffpaul jeffpaul modified the milestones: 0.8.0, 0.7.3 Oct 17, 2022
r-a-y added a commit to r-a-y/bp-two-factor that referenced this pull request Jan 26, 2023
@r-a-y
Remove label and ID injection from JS.
6766c06 
2FA core plugin merged my PR in v0.7.3:
WordPress/two-factor#387

So this is no longer necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@r-a-y r-a-y

Labels
None yet
Projects
None yet
Milestone
0.7.3
Development

Successfully merging this pull request may close these issues.
3 participants
@r-a-y @jeffpaul @kasparsd