WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 19 hours ago

#51899 new enhancement

Improve Application Passwords section layout in user profile

Reported by: SergeyBiryukov Owned by:
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Application Passwords Keywords: has-screenshots early
Focuses: ui, administration Cc:

Description

Background: #42790, #51489.

[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)

51899.png (119.6 KB) - added by SergeyBiryukov 5 months ago.
51984.diff (7.2 KB) - added by butterflymedia 5 months ago.
after.png (37.1 KB) - added by butterflymedia 5 months ago.
user-edit.php.diff (6.2 KB) - added by butterflymedia 3 months ago.
before-5.7.png (40.7 KB) - added by butterflymedia 3 months ago.
after-5.7.png (45.1 KB) - added by butterflymedia 3 months ago.
51899.3.diff (6.3 KB) - added by SergeyBiryukov 3 months ago.
51899.list-table-before.png (29.0 KB) - added by SergeyBiryukov 3 months ago.
51899.3.list-table-after.png (34.6 KB) - added by SergeyBiryukov 3 months ago.

Download all attachments as: .zip

Change History (44)

@SergeyBiryukov
5 months ago

#1 @johnjamesjacoby
5 months ago

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.

#2 @kebbet
5 months 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 @SergeyBiryukov
5 months ago

Related: #51942

Maybe we could move the Application Passwords table to a separate screen, for consistency with other list tables?

#4 @kebbet
5 months ago

#51984 was marked as a duplicate.

#5 @butterflymedia
5 months ago

I have attached my patch from my duplicate ticket #51984.

@butterflymedia
5 months ago

#6 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.7

#7 @hedgefield
4 months 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 months ago

#9 @xkon
4 months 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:

  1. 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)
  2. 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).
  3. 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 :) .

Last edited 4 months ago by xkon (previous) (diff)

This ticket was mentioned in Slack in #design by hedgefield. View the logs.


4 months ago

#11 @TimothyBlynJacobs
4 months 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: @hedgefield
4 months 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: @butterflymedia
4 months 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: @estelaris
4 months 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 @butterflymedia
3 months 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
https://core.trac.wordpress.org/attachment/ticket/51984/before.png

After:

https://core.trac.wordpress.org/attachment/ticket/51984/50a43c241d1a560528dabfa4c86b3aeb.png
https://core.trac.wordpress.org/attachment/ticket/51984/50a43c241d1a560528dabfa4c86b3aeb.png

This ticket was mentioned in Slack in #design by hedgefield. View the logs.


3 months ago

#17 @estelaris
3 months 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.


3 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 months ago

#20 follow-up: @hellofromTonya
3 months ago

  • Keywords commit added

Marking the original patch ready for commit, pending feedback from @SergeyBiryukov.

#21 in reply to: ↑ 20 ; follow-up: @butterflymedia
3 months 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: @hellofromTonya
3 months 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 @butterflymedia
3 months 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.


3 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 months ago

#27 @hellofromTonya
3 months 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 @SergeyBiryukov
3 months 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: @butterflymedia
3 months 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!

Last edited 3 months ago by butterflymedia (previous) (diff)

#30 in reply to: ↑ 29 @SergeyBiryukov
3 months 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.


7 weeks ago

#32 @paaljoachim
7 weeks 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 weeks ago

#34 @chaion07
4 weeks ago

  • Keywords needs-design-feedback removed

Removed the label needs-design-feedback

This ticket was mentioned in Slack in #core-passwords by jeffpaul. View the logs.


19 hours ago

Note: See TracTickets for help on using tickets.