fix: Use nextcloud-vue components for personal info settings by susnux · Pull Request #43488 · nextcloud/server

susnux · 2024-02-09T15:21:35Z

Resolves: [Bug]: Disabled HTML inputs don't appear disabled visually #42498 (comment)

Summary

Use our common styles to make the look consistent.

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

susnux · 2024-02-10T18:59:24Z

Resolved the issue with different input sizes. Needed to refactor all sections to use nextcloud vue inputs. Moreover some other minor issues are now fixed.

And everything is CI tested now.

szaimen

Tested and seems to work. Also looks amazing 🤩
(but didnt review the code)

szaimen · 2024-02-13T10:44:45Z

Only small thing I noticed is that such a phone number seems to be invalid but it should be valid?

(apparently the country code, e.g. +49 instead of 0 is required but it should not be?)

susnux · 2024-02-13T11:43:59Z

@szaimen I did not change anything on the logic there - I just show an error when there is one. Before it silently failed and did not save anything.
So it now at least shows that there is something not right and will not save it.

(libphonenumber is the dependency we use there).

szaimen · 2024-02-13T12:45:38Z

@szaimen I did not change anything on the logic there - I just show an error when there is one. Before it silently failed and did not save anything. So it now at least shows that there is something not right and will not save it.
(libphonenumber is the dependency we use there).

Ah I see. Fine then I guess 👍

Pytal

Other than minor remarks, code looks good!

emoral435

LGTM! Great PR :)

emoral435 · 2024-02-14T14:08:29Z

+ <NcActions :aria-label="actionsLabel" @close="showFederationSettings = false">
+ <template v-if="showFederationSettings">
+ <NcActionButton @click="showFederationSettings = false">


nitpick: Isn't the latter @click redundant, due to the parent NcActions having the same effect on close? It can be removed, because the action gets done twice whenever the button is clicked to close.

No thats two different actions as this is a "submenu".
This one (the button click) is to allow the user to go back in the menu. The close event is to reset the menu to the main menu on close.

Ah, I see. My bad!

…ponents Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

…m handling Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de> Co-authored-by: Pytal <24800714+Pytal@users.noreply.github.com> Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

changes implemented

ShGKme · 2024-02-28T11:16:38Z

+ <FederationControlActions :additional="additional"
+ :additional-value="additionalValue"
+ :handle-additional-scope-change="handleAdditionalScopeChange"
+ :readable="readable"
+ :scope="scope"
+ @update:scope="onUpdateScope" />


NcActions doesn't fully support having NcAction* as non-direct children. It breaks a11y fixes for this component from #43098

Sounds like fixing in components? Can we not provide some registration in actions and inject in NcAction* component to register the different types? So we know for sure which aria role to assign?

susnux added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: settings labels Feb 9, 2024

susnux added this to the Nextcloud 29 milestone Feb 9, 2024

susnux requested review from Pytal, ShGKme and szaimen February 9, 2024 15:21

backportbotBot added the backport-request label Feb 9, 2024

This comment was marked as resolved.
Sign in to view

susnux added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 9, 2024

susnux force-pushed the fix/use-nc-components-account-property-section branch from ee48e44 to 3894f4aCompare February 10, 2024 18:57

susnux added 3. to review Waiting for reviews and removed backport-request 2. developing Work in progress labels Feb 10, 2024

susnux requested a review from ShGKme February 10, 2024 18:58

szaimen approved these changes Feb 13, 2024
View reviewed changes

susnux requested review from JuliaKirschenheuter and emoral435 February 13, 2024 19:40

Pytal approved these changes Feb 14, 2024
View reviewed changes

Comment threadapps/settings/src/components/PersonalInfo/EmailSection/Email.vue Outdated
Comment threadapps/settings/src/components/PersonalInfo/shared/AccountPropertySection.vue Outdated

emoral435 approved these changes Feb 14, 2024
View reviewed changes

susnuxand others added 2 commits February 15, 2024 00:46

fix(settings): Migrate AccountPropertySection to @nextcloud/vue com…
fe58d8a
…ponents Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(settings): Use status states from NcInputField instead of custo…
3e09295
…m handling Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de> Co-authored-by: Pytal <24800714+Pytal@users.noreply.github.com> Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux added 2 commits February 15, 2024 00:47

enh(settings): Add Cypress tests for personal information
e292cf5
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

chore: Compile assets
274867f
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux force-pushed the fix/use-nc-components-account-property-section branch from 8eaf83c to 274867fCompare February 14, 2024 23:54

susnux merged commit 8134559 into masterFeb 15, 2024

susnux deleted the fix/use-nc-components-account-property-section branch February 15, 2024 02:15

ShGKme reviewed Feb 28, 2024
View reviewed changes

ShGKme mentioned this pull request Mar 1, 2024
fix(settings): emails actions a11y and design #43944
Merged
4 tasks

blizzz mentioned this pull request Mar 5, 2024
29.0.0 beta 1 #43991
Merged

Antreesy mentioned this pull request Oct 9, 2024
refactor(chat): import mdi icons nextcloud/spreed#13340
Closed
9 tasks

Uh oh!

Uh oh!

Conversation

susnux commented Feb 9, 2024

Summary

Checklist

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

susnux commented Feb 10, 2024

Uh oh!

szaimen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szaimen commented Feb 13, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

susnux commented Feb 13, 2024

Uh oh!

szaimen commented Feb 13, 2024

Uh oh!

Pytal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

emoral435 left a comment

Choose a reason for hiding this comment

Uh oh!

emoral435Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

susnuxFeb 14, 2024

Choose a reason for hiding this comment

Uh oh!

emoral435Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShGKmeFeb 28, 2024

Choose a reason for hiding this comment

Uh oh!

susnuxFeb 28, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

szaimen left a comment •
edited
Loading

szaimen commented Feb 13, 2024•
edited
Loading