Opened 4 years ago
Last modified 4 months ago
#51899 new enhancement
Improve Application Passwords section layout in user profile
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Application Passwords | Keywords: | has-screenshots has-patch |
Focuses: | ui, administration | Cc: |
Description
[49109] added a new Application Passwords section to user profiles.
@johnjamesjacoby raised a concern in comment:20:ticket:51489:
Is there a compelling reason why the HTML for this feature does not follow the pattern set by every other user profile feature on this page?
This feature would feel much more complete if it did not look as unique and unfamiliar.
The section indeed does not seem to match the rest of the Profile screen, with labels on the left and other elements (inputs, buttons, hints) on the right. See the screenshot, where the Application Passwords section is at the bottom.
Note: By default, application passwords are enabled if your site uses SSL, or if WP_ENVIRONMENT_TYPE
is set to local
. If that is not the case on your testing environment, you can enable the feature with this filter:
add_filter( 'wp_is_application_passwords_available', '__return_true' );
Attachments (9)
Change History (56)
#2
@
4 years ago
I totally agree, and the ”Update Profile” is hidden quite deep into the screen, and with many App passwords its even more confusing.
Might be related -> #51580
#3
@
4 years ago
Related: #51942
Maybe we could move the Application Passwords table to a separate screen, for consistency with other list tables?
#7
@
4 years ago
As much as I too would love to standardize these layouts, and get them to all be vertically laid out instead of the current two-column setup, that would be a big task probably yeah.
If it's difficult to apply that two-column layout to this, I like the idea of making a separate page for it. I can imagine this list could become long in some cases? Paired with the fact that it's so far down the page, it makes sense to split it out for visibility and breathing room. Then it won't matter that it doesn't look like the other settings as much anymore either. But I don't know how much work would be involved in that approach.
This ticket was mentioned in Slack in #design by hellofromtonya. View the logs.
4 years ago
#9
@
4 years ago
I believe that this implementation deserves it's own Page (under Settings or Tools or somewhere..).
I'm already using it daily since all of the "users" on a project that I'm running require an app pass (I'm adding roughly 20 new users daily at the moment and they might have more than 1 application password).
This creates some extra load as I need to create every user (this is fine & part of the process obviously) but then I also have to get into each users Profile and add any application passwords needed. I won't hide of course that I might even forget which user I have already edited so sometimes I need to recheck here and there adding even more clicks haha.
Personally from the way that I've been working with this at least, I would prefer to have 1 single screen for App Passwords that would essentially facilitate:
- Be able to add new & see existing an Application Passwords for my account,similar to what exists in Profile edit (any user with appropriate access)
- A list table so I can easily see which users are using app passwords, I can either way see it via profile edit already it would just make my life way easier (admin only).
- An extra form that would consist of a dropdown option or search box (list of usernames basically) and application name to create a new Application Password for any user in an easier way than going to each profile of any given user (admin only).
Is this something that we could consider maybe just in case it would be more helpful this way?
To avoid losing scope on this ticket also do tell me if you want me to pass these thoughts on a new ticket and maybe add some screenshots for an easier view of this proposal :) .
This ticket was mentioned in Slack in #design by hedgefield. View the logs.
4 years ago
#11
@
4 years ago
I believe that this implementation deserves it's own Page (under Settings or Tools or somewhere..).
I think if we put it on a separate page, it should probably go under Users.
#12
follow-up:
↓ 13
@
4 years ago
I think there's a fair usecase to consider for having all app password management on one page, like @xkon suggests. The 5.7 beta deadline is looming at the end of this week though, and we can't add new 5.7 features after that - however we CAN iterate on beta features after the beta 1 deadline. So my suggestion would be to commit this in the next few days so that we're sure it's included in 5.7, and then iterate on the design and exact location in the admin in the next couple of weeks. I see @butterflymedia already tackled the two-column layout, @SergeyBiryukov is there anything more you need to get this ready for commit?
#13
in reply to:
↑ 12
;
follow-up:
↓ 14
@
4 years ago
Replying to hedgefield:
I see @butterflymedia already tackled the two-column layout, @SergeyBiryukov is there anything more you need to get this ready for commit?
I can provide more visual fixes if needed, just to have this in line with the USers page. Next iteration could move it to a separate page.
#14
in reply to:
↑ 13
;
follow-up:
↓ 15
@
4 years ago
Replying to butterflymedia:
I can provide more visual fixes if needed, just to have this in line with the USers page. Next iteration could move it to a separate page.
Could you please provide screenshots? The design team will review the ticket today.
#15
in reply to:
↑ 14
@
4 years ago
Replying to estelaris:
Replying to butterflymedia:
I can provide more visual fixes if needed, just to have this in line with the USers page. Next iteration could move it to a separate page.
Could you please provide screenshots? The design team will review the ticket today.
I already have them in #51984:
Before:
https://core.trac.wordpress.org/attachment/ticket/51984/before.png
After:
https://core.trac.wordpress.org/attachment/ticket/51984/50a43c241d1a560528dabfa4c86b3aeb.png
This ticket was mentioned in Slack in #design by hedgefield. View the logs.
4 years ago
#17
@
4 years ago
- Keywords needs-design-feedback added; needs-design removed
Design team is ok with the after look. As @hedgefield proposes, let's commit as is and if we need to iterate on the design, we can do that again.
Added the "needs design feedback" to review it again after committing
@SergeyBiryukov what else do you need to commit this?
This ticket was mentioned in Slack in #core by jjj. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#20
follow-up:
↓ 21
@
4 years ago
- Keywords commit added
Marking the original patch ready for commit
, pending feedback from @SergeyBiryukov.
#21
in reply to:
↑ 20
;
follow-up:
↓ 22
@
4 years ago
Replying to hellofromTonya:
Marking the original patch ready for
commit
, pending feedback from @SergeyBiryukov.
When you say "original" do you mean the patch I provided (initially in #51984)?
It would be my very first patch :)
#22
in reply to:
↑ 21
;
follow-up:
↓ 23
@
4 years ago
Replying to butterflymedia:
Replying to hellofromTonya:
Marking the original patch ready for
commit
, pending feedback from @SergeyBiryukov.
When you say "original" do you mean the patch I provided (initially in #51984)?
It would be my very first patch :)
Hello @butterflymedia,
I'm referring to the patch 51984.diff attached to this ticket. WooHoo for your very first patch!
#23
in reply to:
↑ 22
@
4 years ago
Replying to hellofromTonya:
Hello @butterflymedia,
I'm referring to the patch 51984.diff attached to this ticket. WooHoo for your very first patch!
Thanks!
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#27
@
4 years ago
- Keywords early added; commit removed
- Milestone changed from 5.7 to 5.8
5.7 Beta 1 is starting now. We ran out of time to get the original patch into a committable state and then committed. Let's get this done early
in 5.8.
#28
@
4 years ago
- Keywords needs-refresh added
Thanks for the patch! However, it appears to have some formatting issues:
- It's reversed, so the removed lines are actually new, and vice versa.
- It replaces tabs with spaces, probably via auto-formatting options in your editor.
- It also needs a refresh after [50006].
#29
follow-up:
↓ 30
@
4 years ago
Done!
I have updated my repo to the latest version - 5.7-beta1-50172 - and re-applied my fixes (see attached diff).
I have also switched to tabs (instead of spaces).
I have also attached a before/after screenshot.
Cheers!
#30
in reply to:
↑ 29
@
4 years ago
- Keywords needs-refresh removed
Replying to butterflymedia:
I have updated my repo to the latest version - 5.7-beta1-50172 - and re-applied my fixes (see attached diff).
Thanks! user-edit.php.diff does apply cleanly.
51899.3.diff fixes a few WPCS issues.
This might need a few more adjustments:
- The padding in the list table headers and cells is not the same after the patch, see 51899.list-table-before.png and 51899.3.list-table-after.png
- The duplicate "Application Passwords" heading seems redundant. Perhaps this section could be merged with the previous "Account Management" section? Alternatively, the second instance of the heading (the smaller one) could be changed to something else.
This ticket was mentioned in Slack in #design by chaion07. View the logs.
4 years ago
#32
@
4 years ago
The above screenshot looks good. It was brought up during a design triage.
As you mention @SergeyBiryukov it needs some padding.
I agree on the duplicate "Application Passwords" heading seems redundant.
It would be nice to cleanup the above things and then post another screenshot.
Thanks!
This ticket was mentioned in Slack in #design by chaion07. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-passwords by jeffpaul. View the logs.
4 years ago
#36
@
4 years ago
- Milestone changed from 5.8 to 5.9
This one still needs a refresh to address some of the points mentioned in the previous few comments. Since feature freeze is today, I'm going to punt to 5.9
.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#38
@
3 years ago
Recommend to swap out:
if ( isset( $data['post_parent'] ) && $data['post_parent'] ) {
...for...
if ( ! empty( $data['post_parent'] ) ) {
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#41
@
3 years ago
- Keywords early removed
As per today's bug scrub, I'm removing the early
keyword as we don't really need this early in the release cycle :)
#42
@
3 years ago
- Keywords has-patch needs-refresh added
Marking as having a patch but also needs a refresh with updates noted previously.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#45
@
3 years ago
- Milestone changed from 5.9 to Future Release
As per today's bug scrub, let's move this ticket to Future release
. We need to address the feedback received in comment 30.
Happy to move it back to 5.9 if a new patch is ready to go in the very next few days.
This ticket was mentioned in PR #7640 on WordPress/wordpress-develop by @thrijith.
4 months ago
#46
- Keywords needs-refresh removed
This PR refreshes the patch provided in https://core.trac.wordpress.org/ticket/51899#comment:30 and also adds style changes requested in the comment.
Trac ticket: https://core.trac.wordpress.org/ticket/51899
In addition, this feature only works with JavaScript enabled, because it adds a hack to avoid buttons submitting the form that it’s inside of.
I spent some time attempting to patch this minimally a few weeks ago, and concluded that it just needs to be redesigned completely.
WordPress styling does not have support for nested tables – so a List Table inside a Form Table is rightfully not a very good approach even if we wanted to try. And if these passwords stop using the List Table API, that starts to become a major refactor and redesign project that probably requires involving more stakeholders.
In comparison, many of the other WordPress Admin pages have also become a mish-mash of unmatched sections (Discussion settings is the worst) so the silver lining is that Profiles now looks just as mixed-up as the others? 🥴
My WP User Profiles plugin uses its own markup and JavaScript to implement application passwords because this feature is not abstracted in such a way to make it easy to repurpose (the way that the password generator is, for example).
When BuddyPress or bbPress decide to implement this for their theme-side profiles, they will also need their own bespoke implementations.
I’m all for improving this, but now that’s landed like it is, it’s a bit of a nightmare to undo.