-
Notifications
You must be signed in to change notification settings - Fork 9
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
Show UI on front end #15
Comments
I think this is the current best place for it.
I think it would make sense to split the "Account" subsection that has Email/Password onto a new tab, and list that along with 2FA on that page. Alternatively, I guess just starting off with 2FA on it's own dedicated tab would make sense for now. This is where I think it should live, although I admit it's not currently where it makes the most sense. The reason this isn't the de-facto login-management page is because previously it didn't have Memcache, so you couldn't change your password here. There's nothing stopping us removing Password/Email management from Support and moving it to Profiles now, Realistically, The Forum profile tab should not really need be used by regular users. |
Agree that a dedicated page/tab would make sense for the password/2FA settings - the current password flow feels a little awkward but maybe something to improve at a later stage? There is some good examples of either checkbox security (WP.com), and grouping of similar items (Github.com), but again perhaps something to push later. |
@dd32 I assigned this to you because it looks like you've already got some code written, but feel free to un-assign if that's wrong. I just wanted to avoid someone else picking it up and duplicating efforts. |
I've cleared the assignment; Only because I haven't written any code. The integration into the bbPress user-edit screen is done by bbPress itself, as it displays user-profile page custom sections on the front-end. The two-factor plugin mostly works there, although not 100% as the plugin is tied to the notion it's on the wp-admin/profile.php screen. |
As noted, this depends upon WordPress/two-factor#261 but also, as noted in #5 I anticipate we'll need to customise whatever approach the two-factor plugin takes significantly for our own UI if we were to have reasonably good UX. |
Ah, I didn't realize it was already doing that 👍🏻 IMO it'd be ideal if we can make most of the changes from #5 upstream, to benefit everyone and avoid having to maintain a forked experience. We'll probably want to do some customization no matter what though, even if it's just CSS to fit the Support theme. |
Even if we don't have time to contribute the visual stuff upstream, at least having a stable API upstream for saving options etc would make it safe to customize the UI. |
These should be updated as part of this issue. wporg-two-factor/wporg-two-factor.php Lines 130 to 131 in a10a26d
wporg-two-factor/wporg-two-factor.php Lines 139 to 148 in a10a26d
|
I like that 👍🏻 Ideally we'd go farther and unify the two profiles, but only if we have time. |
As a follow up to this, I would like to say I was wrong. BuddyPress has a UI for changing email/password but it's not viable for our usage at this point in time without a lot of work. When we redesign the profiles, rebuilding it as a block theme, we'll likely remove a lot of the BuddyPress templating and build our own minimal UI that does what we need. At that point in time we can Unify profiles. |
Started looking at this - it's not without it's challenges, and firstly I'm going to recreate a flow that might be appropriate within a sub-page in wp-admin, with the intent of making the methods of save etc callable. Taking inspiration from some work done a while ago in https://github.com/WordPress/two-factor/blob/add/totp-ajax/. The more I look, the more complications I seem to find, which i'll note later. |
Does it look like something that could be broken into multiple issues that could be worked on in parallel? This is probably the biggest chunk of work left, so working in parallel would help us meet the deadline. If you stub out the basics and merge that, then all 3 of us could be working on filling in the gaps and polishing different parts of it. |
Yes - let me check back in with where I got, then I'll start doing some committing and create some other issues. Primarily I've been looking at the backup codes and adding some additional logic/functionality. |
I created #26 for the task mentioned above about moving the Account settings to a separate screen. |
Based on our other discussions, I committed stubs for the UI in d457c55 (related: a697745 and 77ddf2b). That will hopefully give us something tangible to start building on, but it's still rough so feel free to propose big changes. I also created issues #27, #28, #29, #30, and #31, so that we can work on individual parts in parallel with a minimum of merge conflicts. Is there anything left to do in this issue, or is it superseded by the other ones now? |
Closing since it seems like this is done/superseded now, but feel free to reopen if I missed something. |
Part of #5
We know that we want to show the UI on the front end, but need to pick a location and render the settings there.
https://wordpress.org/support/users/profile/edit/
this is where passwords are set, so it could make sense to group it with this. There's already a lot on that page, though.https://profiles.wordpress.org/profile/edit/group/1
- This makes less sense to me than the Support profile, but is another place where account settings live.login.w.org/settings
(or something like that) - This doesn't make as much sense to me, since we already have 2 other profile screens, and this would add a 3rd.Regardless of which domain it goes on, I think it'd be best to create a dedicated page, rather than appending it to an existing page. 2FA is a relatively complex thing to setup, so having it's own page would be a better UX IMO.
This depends on WordPress/two-factor#261. If we just built own custom UI that would be fragile, and upstream changes could silently introduce conflicts/vulnerabilities.
The text was updated successfully, but these errors were encountered: