Replace plain input fields with NcTextField fields and NcMultiSelect …

[JuliaKirschenheuter]

Replace plain input fields with NcTextField fields and NcMultiSelect fields with NcSelect fields

• Resolves: https://github.com/nextcloud/server/issues/38145
• Resolves: https://github.com/nextcloud/server/issues/36976

Summary

Before:

After:

Also added notifications after displaty name, password and email were changed.

TODO

• [x] Make possible to create a new group for availableGroups
• [x] Create item limitation for subAdminsGroups and availableGroups NcSelect (:limit prop on NcSelect is just not working)

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• [ ] Tests (unit, integration, api and/or acceptance) are included
• [x] 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)

[JuliaKirschenheuter]

@Pytal @susnux @artonge how can i resolve this todo point: Make possible to create a new group for availableGroups? and this one: Create item limitation for subAdminsGroups and availableGroups NcSelect (:limit prop on NcSelect is just not working)?

[JuliaKirschenheuter]

@jancborchardt @nimishavijay is it ok design-wise?

[Pytal]

@Pytal @susnux @artonge how can i resolve this todo point: Make possible to create a new group for availableGroups?

I believe the name needs to be destructured with something like 👇

async createGroup({ name: gid }) {

and this one: Create item limitation for subAdminsGroups and availableGroups NcSelect (:limit prop on NcSelect is just not working)?

Since limit for NcSelect applies to dropdown options rather than selected options, as seen in vue-multiselect, you may try https://vue-select.org/guide/selectable.html#limiting-the-number-of-selections

[JuliaKirschenheuter]

Since limit for NcSelect applies to dropdown options rather than selected options, as seen in vue-multiselect, you may try https://vue-select.org/guide/selectable.html#limiting-the-number-of-selections

Thanks! I've tried it before too, but used wrong attr: options instead of value. It works now, but we probably should do something with styles (but not in this PR):

[JuliaKirschenheuter]

Regarding adding a new group there are several problems:

1. option:selected returns array of all selected groups. Each new selected group - is a last element of an array.
2. option:created doesn't allow to use a result like a value of new option. Based on this event, it allows creating a new group on the server, but it won't be added to the list of group options from the server itself. Just { name } will be added.
3. create-option is used for creating new options but that works only for sync input of each symbol. It is not possible to use it for async adding a group on the server.
4. After option:created has been fired an option:selected event will follow. But the result of option:selected would have a value like { name } even if we would put manually to event option:created a list of a groups.

Thanks @ShGKme for investigation!

But still have no idea how to fix that problem ;(

[ShGKme]

Some notes:

1 - option:selected returns array of all selected groups

Not a big problem, just a difference with NcSelect.

We can either call addUserGroup with the last element in the template: @option:selected="addUserGroup($event.at(-1))"... ... or update addUserGroup/create a new method to handle an array.

2 - option:created doesn't allow to use a result like a value of new option

But it won't be added to the list of group options from the server itself

Sorry, I was a bit not accurate. I meant, the internal selected list (selected groups) in NcSelect has an internal { name: <inputted> } value.

Combination of 2 + 3 + 4 - no support of async tag creation

The problem is that @option:selected and its handler addUserGroup is triggered also for a new group before it was created on the server with NcSelect's internal { name: <inputted> } value.

---

So if don't use v-model, all we need is to ignore this being created value in the option:selected handler.

The simplest solution is to check if it is a real group object, aka

async addUserGroup(group) {
  if (group.id === undefined) {
    // This is NcSelect's internal value for a new inputted group name
    // Ignore
    return
  }
  // Add the user to the group
}

This solution should work but looks dirty and implicit. I'd explicitly mark the being inputted option as temp/being created using create-option.

<NcSelect
  :create-option="(value) => ({ name: value, isCreating: true })"
  @option:selected="addGroup($event.at(-1))"
/>

async addUserGroup(group) {
  if (group.isCreating) {
    // This is NcSelect's internal value for a new inputted group name
    // Ignore
    return
  }
  // Add the user to the group
}

An alternative solution could be: to drop using option:created and instead of ignorance create the group in the option:selected handler. But the core idea is the same.

[nimishavijay]

@JuliaKirschenheuter it looks great! Much better now :)

Some minor enhancements:

• [ ] decrease the width of the "Quota", "Language", and "Manager" columns (by around 3-4px each), and increase the width of the email column (by around 12-14px)
• [ ] Show the read only items even in edit mode instead of hiding them. They can be shown in the same way as they are shown in view mode, so just as plain text.

They can also be in a follow up PR! The changes here look good! :)

[JuliaKirschenheuter]

Dear @nimishavijay

I'm very sorry, could you please make a new Issue for this 2 improvements? This implementation has already taken more then planned ;( If it would't be so, i would fix them here.

I've created an issue with improvements: https://github.com/nextcloud/server/issues/39002

[susnux]

Drone error is related because it looks for the multiselect instead of the select, so the acceptance tests need to be adjusted, this needs to be ported to NcSelect:

https://github.com/nextcloud/server/blob/4cfab4b838ed40dec200f7673992009896c69f16/tests/acceptance/features/bootstrap/UsersSettingsContext.php#L155

https://github.com/nextcloud/server/blob/4cfab4b838ed40dec200f7673992009896c69f16/tests/acceptance/features/bootstrap/UsersSettingsContext.php#L120

[JuliaKirschenheuter]

This does not look right, is this known?

yes, it is known. I've tried to fix that problem here https://github.com/nextcloud/server/pull/37870, but still had problems with a table itself and had to revert this PR. But that is known and will be fixed.

[susnux]

This does not look right, is this known? Probably a matter of z-index somewhere.

I guess this rule: https://github.com/nextcloud/server/blob/cc1498b225bf1d70bd81854f8941f333fcd61209/apps/settings/src/components/Users/UserRow.vue#L807

[JuliaKirschenheuter]

This does not look right, is this known? Probably a matter of z-index somewhere.

I guess this rule:

https://github.com/nextcloud/server/blob/cc1498b225bf1d70bd81854f8941f333fcd61209/apps/settings/src/components/Users/UserRow.vue#L807

no, unfortunately not. Please look in master, there is a same problem.

[JuliaKirschenheuter]

@susnux, @artonge could you please help me to fix integration tests?

[susnux]

There is still the issue with the password change, I verified it works locally. I think the issue is that it does not wait for the password to change:

The loading icon for user user0 was not found after 1 seconds, assumming that it was shown and hidden again before the check started and continuing

(Maybe we should switch the settings tests also to cypress in the future, as it seems to be easier to adjust.)