User management fields#2921
Conversation
cairocoder01
left a comment
cairocoder01
left a comment
There was a problem hiding this comment.
This is a great start. See the specific code comments, but there are also some functional issues:
- When using the new user form, the submit doesn't work because it returns an API error
- When changing fields of an existing user, they don't automatically save like they used to. We need to hook up the code to capture change events and trigger the API requests
- Those will probably require js changes. Try to find the js code that was specific to the old form fields and remove it as you implement the new code for the components.
| 'type' => 'text', | ||
| 'default' => '', | ||
| ] | ||
| ], [], [ 'placeholder' => 'Nickname (Display Name)' ] ) ?> |
There was a problem hiding this comment.
This value needs to be localized so it's translated in other languages
| 'default' => '', | ||
| 'required' => true, | ||
| ] | ||
| ], [], [ 'placeholder' => 'Email' ] ) ?> |
There was a problem hiding this comment.
Localize placeholder
| 'type' => 'text', | ||
| 'default' => '', | ||
| ] | ||
| ], [], [ 'placeholder' => 'Username' ] ) ?> |
There was a problem hiding this comment.
Localize placeholder
| 'type' => 'text', | ||
| 'default' => '', | ||
| ] | ||
| ], [], [ 'placeholder' => 'Password' ] ) ?> |
There was a problem hiding this comment.
Localize placeholder
| 'type' => 'text', | ||
| 'default' => '', | ||
| ] | ||
| ], [], [ 'placeholder' => 'First Name' ] ) ?> |
There was a problem hiding this comment.
Localize placeholder
| 'type' => 'text', | ||
| 'default' => '', | ||
| ] | ||
| ], [], [ 'placeholder' => 'Last Name' ] ) ?> |
There was a problem hiding this comment.
Localize placeholder
| <dt-toggle id="app_state_${app_key}" | ||
| class="app-state-switches" type="checkbox" |
There was a problem hiding this comment.
When I toggle one of these, I get a js error that the request failed. But it seems to trigger the request twice and it was the second of the two that failed. Specifically, it's when I uncheck one of these toggles that it happens.
| <p> | ||
| <?php esc_html_e( 'Display Name', 'disciple_tools' ); ?> | ||
| <span id="<?php echo esc_html( 'update_display_name' ); ?>-spinner" class="loading-spinner"></span> |
There was a problem hiding this comment.
We should replace these labels by setting the label attribute of the components. And the loading spinner should use the loading icon/attribute of the component
| <dl> | ||
| <dt><label for="eman"><?php esc_html_e( 'Display Name', 'disciple_tools' ); ?></label></dt> | ||
| <dd><input type="text" class="input" id="eman" placeholder="<?php esc_html_e( 'Nickname (Display Name)', 'disciple_tools' ); ?>" required autocomplete="off" /> </dd> | ||
| <dt><label for="liame"><?php esc_html_e( 'Email', 'disciple_tools' ); ?></label></dt> | ||
| <dd><input type="email" class="input" id="liame" placeholder="<?php esc_html_e( 'Email', 'disciple_tools' ); ?>" required autocomplete="off" /> </dd> | ||
| <dt></dt> | ||
| <dd> |
There was a problem hiding this comment.
We can get rid of this dl/dt/dd structure. A dl is a definition list for definition terms (dt) and descriptions (dd). It works well for labels and values, but those are contained within the components now. We can just put the components side-by-side (except maybe the hidden ones, which can stay in their own div).
3 participants
@cairocoder01 This resolves #2847 to replace user management fields w dt-web-components.
Please specifically look at the syntax and formatting of my changes to make sure it's what you're looking for!