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(NewCollectiveModal): Mention that own user is part of collective #1003

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

mejo-
Copy link
Member

@mejo- mejo- commented Nov 9, 2023

People got confused by searching their own user.

🖼️ Screenshots

🏚️ Before 🏡 After
screenshot recording1

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits

@@ -87,6 +87,7 @@
<MemberPicker :show-selection="true"
:selected-members="selectedMembers"
:on-click-searched="onClickSearched"
:empty-content-hint="t('collectives', 'Your user is added automatically.')"
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about the wording. Maybe "As creator, you're already added." is better? 🤔

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Instead of as a message, we could show the owner's account directly below in the list like added accounts do? That would make it more obvious than a line of text.

Slightly related: We want to move from the wording "user" to either "account" or "person", could be replaced in the search field.

Copy link

cypress bot commented Nov 9, 2023

1 flaky test on run #1270 ↗︎

0 253 0 0 Flakiness 1

Details:

fix(NewCollectiveModal): Mention that own user is part of collective
Project: Collectives Commit: bafc785c02
Status: Passed Duration: 10:49 💡
Started: Nov 10, 2023 10:40 AM Ended: Nov 10, 2023 10:51 AM
Flakiness  cypress/e2e/pages.spec.js • 1 flaky test • Nextcloud stable27

View
Output

Test Artifacts
Page > Editing a page > Lists attachments for the page and allows restore Test Replay Screenshots

Review all test suite changes for PR #1003 ↗︎

@mejo-
Copy link
Member Author

mejo- commented Nov 9, 2023

Thanks @jancborchardt, much better. Implemented now, see the screencast above. I also changed "users" to "accounts" where appropriate.

@mejo- mejo- force-pushed the fix/create_collective_hint branch 2 times, most recently from 88757c7 to 6b4b1ed Compare November 9, 2023 16:39
@mejo- mejo- requested a review from jancborchardt November 9, 2023 16:44
@mejo-
Copy link
Member Author

mejo- commented Nov 9, 2023

I wonder whether it would further help to change the "Create" button to something like "Create without adding people" in case no other members were selected.

@juliusknorr
Copy link
Member

Small addition, we could preserve the previously entered collective name in the dialog title to make it more obvious what this is about, e.g. "Add people to fox" instead of just "Add people".

Overall i find the different usages of account/people/members a bit confusing.

Screenshot 2023-11-09 at 19 56 28 Screenshot 2023-11-09 at 19 55 35 Screenshot 2023-11-09 at 19 55 54

Maybe we can somehow unify that? Pick one high level word (members) and a fixed set for the actual membership entities (account, group, circle)?

@mejo-
Copy link
Member Author

mejo- commented Nov 10, 2023

Overall i find the different usages of account/people/members a bit confusing.
Maybe we can somehow unify that? Pick one high level word (members) and a fixed set for the actual membership entities (account, group, circle)?

I agree, thanks for bringing this up. Your suggestion makes sense: account, group, circle for the different entities and "members" for all of them. So I'd replace "Add people" with "Add members" and search for more occurrences of "users" to be replaced by "accounts". Agreed?

@mejo- mejo- force-pushed the fix/create_collective_hint branch from 6b4b1ed to a098f0b Compare November 10, 2023 10:26
People got confused by searching their own user.

Signed-off-by: Jonas <[email protected]>
…ers"

* Mention collective name in title: "Add members to <name>".
* Change "Create" button to "Create without members" if no members are
  picked.

Signed-off-by: Jonas <[email protected]>
@mejo- mejo- force-pushed the fix/create_collective_hint branch from a098f0b to bafc785 Compare November 10, 2023 10:29
@mejo-
Copy link
Member Author

mejo- commented Nov 10, 2023

Small addition, we could preserve the previously entered collective name in the dialog title to make it more obvious what this is about, e.g. "Add people to fox" instead of just "Add people".

Great idea. Implemented now (using "Add members to "). See updated screencast above.

I agree, thanks for bringing this up. Your suggestion makes sense: account, group, circle for the different entities and "members" for all of them. So I'd replace "Add people" with "Add members" and search for more occurrences of "users" to be replaced by "accounts". Agreed?

Also implemented. See updated screencast above.

@mejo- mejo- merged commit e2b26f2 into main Nov 13, 2023
49 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix/create_collective_hint branch November 13, 2023 11:06
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.

3 participants