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

Add label to "default quota" multiselect #38075

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented May 4, 2023

Summary

Correct label for "default quota" multiselect. No visual changes.

Before After
Screenshot from 2023-05-04 15-39-41 Screenshot from 2023-05-09 10-08-18

Checklist

@JuliaKirschenheuter JuliaKirschenheuter added the 3. to review Waiting for reviews label May 4, 2023
@JuliaKirschenheuter JuliaKirschenheuter self-assigned this May 4, 2023
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we take the opportunity to switch to NcSelect?
https://nextcloud-vue-components.netlify.app/#/Components/NcSelect

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from f29e037 to 0060a85 Compare May 5, 2023 10:42
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as draft May 5, 2023 10:43
@JuliaKirschenheuter JuliaKirschenheuter added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 5, 2023
@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented May 5, 2023

Can we take the opportunity to switch to NcSelect? https://nextcloud-vue-components.netlify.app/#/Components/NcSelect

I've converted NcMultiselect to NcSelect but have 2 problems:

  1. :close-on-select="false" seems not to work. I would like to record this odd behavior but unfortunately can't. Somehow it si not possible.
  2. Have a problem with 'unlimited', If i'm choosing it -> 'none' will be shown. After page reloading i get correct 'unlimited'.

@Pytal , @susnux could you please check out this state and help me with correction? Thanks a lot!

@susnux
Copy link
Contributor

susnux commented May 5, 2023

2. Have a problem with 'unlimited', If i'm choosing it -> 'none' will be shown. After page reloading i get correct 'unlimited'.

Because the setDefaultQuota sets the label to the id which is undefined, one had to defaultQuota in this case to unlimitedQuota. (I hope it was ok to push this directly? Feel free to squash / revert :) )

apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
@susnux
Copy link
Contributor

susnux commented May 5, 2023

:close-on-select="false" seems not to work. I would like to record this odd behavior but unfortunately can't. Somehow it si not possible.

This works, but is no the reason for the issue. The settings close as soon as you select a quota because the dropdown menu is not rendered within the settings, but the body so the click outside toggle of NcAppNavigationSettings triggers and it closes.

See comment :)

@JuliaKirschenheuter
Copy link
Contributor Author

Because the setDefaultQuota sets the label to the id which is undefined, one had to defaultQuota in this case to unlimitedQuota. (I hope it was ok to push this directly? Feel free to squash / revert :) )

Thank you! Absolutely fine! Works

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from e005f5b to 784059f Compare May 9, 2023 08:02
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review May 9, 2023 08:03
apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
@JuliaKirschenheuter JuliaKirschenheuter added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 10, 2023
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@artonge
Copy link
Contributor

artonge commented May 10, 2023

Have you checked whether the quota is actually changed in the DB?

@JuliaKirschenheuter
Copy link
Contributor Author

Have you checked whether the quota is actually changed in the DB?

I've checked it by reloading a page and by expecting a same value as set before. Should i double check in DB? Which table should i check for it? Thank you!

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from 10b47b4 to 18ec8a6 Compare May 10, 2023 13:43
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently adding custom quotas is broken :/

apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved
@JuliaKirschenheuter JuliaKirschenheuter added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 11, 2023
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as draft May 11, 2023 15:02
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from ba3a6a4 to 1daf2b7 Compare May 16, 2023 15:56
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review May 16, 2023 15:58
@JuliaKirschenheuter JuliaKirschenheuter added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 16, 2023
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from 1daf2b7 to e4f6068 Compare May 19, 2023 07:19
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides that two minor comments everything is fine 😃

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from e4f6068 to b79b770 Compare May 23, 2023 11:02
Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch from b79b770 to 1c90d7f Compare May 23, 2023 12:18
@JuliaKirschenheuter JuliaKirschenheuter merged commit ea46959 into master May 23, 2023
37 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/36923-The_Default_quota_input_field_in_the_Settings_section_is_not_programmatically_linked_to_its_visual_label branch May 23, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
4 participants