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.

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

Fix input for groups validation in new user form #29968

Merged
merged 2 commits into from
Dec 7, 2021

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Nov 29, 2021

Fixes #28992
Fixes #29063

The hidden input used for form validation was not actually hidden since Nextcloud 22, as the DOM structure changed to show a dialog rather than adding a row on top of the list when adding new users, so the CSS rules no longer matched.

Before:
Subadmin-New-User-Form-Empty-Before
Subadmin-New-User-Form-Filled-Before

After:
Subadmin-New-User-Form-Empty-After
Subadmin-New-User-Form-Filled-After
Subadmin-New-User-Form-Validation-After
Last screenshot is slightly outdated, as the list of options is no longer shown behind the Please fill out this field message

@danxuliu danxuliu added this to the Nextcloud 24 milestone Nov 29, 2021
@danxuliu danxuliu requested review from skjnldsv, a team, PVince81 and juliusknorr and removed request for a team November 29, 2021 19:29
@danxuliu
Copy link
Member Author

/backport to stable23

@danxuliu
Copy link
Member Author

/backport to stable22

@danxuliu danxuliu force-pushed the fix-input-for-groups-validation-in-new-user-form branch 2 times, most recently from 011b747 to dc4bb20 Compare December 2, 2021 07:08
@danxuliu danxuliu force-pushed the fix-input-for-groups-validation-in-new-user-form branch from dc4bb20 to 3fe2b57 Compare December 2, 2021 08:25
@danxuliu
Copy link
Member Author

danxuliu commented Dec 2, 2021

When I initially worked on this I had missed the existing but no longer applied CSS rules 🤦 With them it is possible to hide the hidden input.

Note that in case of a validation error the hidden input will be the one focused, and then a click or a tab will be needed to focus the Multiselect component. However, although this is slightly inconsistent with how the other fields work after some testing I think that in this case it is better; if the input element inside the Multiselect component was the one focused instead (like in the original implementation of this pull request) the Please fill out this field could overlap the first element in the list of possible groups (as can be seen in the last screenshot).

The width of the hidden field needs to be set to 0 to prevent it from getting the first click after a validation error in Firefox. The drawback is that, in Chromium, the message about having to complete the field is not based on the left position of the field like in Firefox, but on its width, so the alignment of the message is not the same as in other fields in the dialog :-(

@danxuliu danxuliu force-pushed the fix-input-for-groups-validation-in-new-user-form branch from 3fe2b57 to 8c828a2 Compare December 2, 2021 08:28
@artonge
Copy link
Contributor

artonge commented Dec 7, 2021

/rebase

The hidden input used for form validation was not actually hidden since
Nextcloud 22, as the DOM structure changed to show a dialog rather than
adding a row on top of the list when adding new users, so the CSS rules
no longer matched.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Since Nextcloud 22 the "#new-user" rules had no effect, as the DOM
structure changed to show a dialog rather than adding a row on top of
the list when adding new users.

Similarly, the z-index was no longer needed, as there will be no
"new-user" row that could overlap. Moreover, the z-index was set even
higher (100) in another rule still active.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu force-pushed the fix-input-for-groups-validation-in-new-user-form branch from 8c828a2 to b7cbba0 Compare December 7, 2021 17:05
@artonge artonge merged commit 6c0036a into master Dec 7, 2021
@artonge artonge deleted the fix-input-for-groups-validation-in-new-user-form branch December 7, 2021 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Group admin cannot create new users Subadmin user-add and strange required input fields
3 participants