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

Replace "transfer" notion with "convert" notion #1228

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

Jerome-Herbinet
Copy link
Member

@Jerome-Herbinet Jerome-Herbinet commented Aug 19, 2024

Replace "transfer" notion with "convert" notion

Test from reviewers needed.

Fixes: #1221 (except menu entries order which is IMO another development … that I can't do myself)

@Jerome-Herbinet Jerome-Herbinet added enhancement New feature or request design Related to the design 2. developing Work in progress papercut Small issues that doesn't break the ux/ui labels Aug 19, 2024
@Jerome-Herbinet Jerome-Herbinet force-pushed the Jerome-Herbinet-transfer-to-convert branch from 34b26cb to d2954e2 Compare August 19, 2024 12:17
@Jerome-Herbinet Jerome-Herbinet added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 19, 2024
@Jerome-Herbinet
Copy link
Member Author

@Pytal (you or another possible reviewer) I need you to test ; thanks by advance.

lib/Notifications/Notifier.php Outdated Show resolved Hide resolved
lib/Notifications/Notifier.php Outdated Show resolved Hide resolved
src/components/TransferGuestDialog.vue Outdated Show resolved Hide resolved
src/components/TransferGuestDialog.vue Outdated Show resolved Hide resolved
src/users.ts Outdated Show resolved Hide resolved
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.

Seems good to me – I only don’t know how the more complex constructions like this one would come out:

$l->t('Conversion of guest {guest} account to {user} account completed'),

The previous simpler way without the 2 "account" wordings seems nicer? But @Jerome-Herbinet if you have examples of before/after how it will actually come out with example wording that would be nice.

@Jerome-Herbinet
Copy link
Member Author

Seems good to me – I only don’t know how the more complex constructions like this one would come out:

$l->t('Conversion of guest {guest} account to {user} account completed'),

The previous simpler way without the 2 "account" wordings seems nicer? But @Jerome-Herbinet if you have examples of before/after how it will actually come out with example wording that would be nice.

@jancborchardt you are right ; the sentence is a bit complex, but I think that giving enough details is necessary in this process. I would even prefer, even it's longer (guest vs. regular) :

Conversion of {guest} guest account to {user} regular account completed

... and to apply to the other wordings.

@jancborchardt
Copy link
Member

@Pytal since you are more pro at English, what do you suggest? :D

I would prefer to go with a simple and concise sentence as that often is more understandable.

@Pytal
Copy link
Member

Pytal commented Aug 20, 2024

Both ways have valid points I think — considering the messages in isolation and without assuming contextual knowledge I would say the more explicit messaging makes sense

@Jerome-Herbinet Jerome-Herbinet force-pushed the Jerome-Herbinet-transfer-to-convert branch from 7c8c65d to d6334bd Compare August 21, 2024 08:33
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.

@Jerome-Herbinet could you still show how these would come out in practice?

$l->t('Failed to convert guest {guest} account to {user} account')
$l->t('Conversion of guest {guest} account to {user} account completed')

@Jerome-Herbinet Jerome-Herbinet force-pushed the Jerome-Herbinet-transfer-to-convert branch from d6334bd to c1c3176 Compare August 22, 2024 13:28
@Jerome-Herbinet
Copy link
Member Author

'Conversion of guest {guest} account to {user} account completed'

@Jerome-Herbinet could you still show how these would come out in practice?

$l->t('Failed to convert guest {guest} account to {user} account')
$l->t('Conversion of guest {guest} account to {user} account completed')

I won't be able to test it @jancborchardt

@jancborchardt
Copy link
Member

@Pytal do you know what will come out of those sentences? :D It would be important to know so we can decide on the proper wording.

@Pytal
Copy link
Member

Pytal commented Aug 26, 2024

@jancborchardt looks like this
image

@Pytal
Copy link
Member

Pytal commented Aug 26, 2024

With the longer version "Conversion of guest account, {guest}, to regular account, {user}, completed" would be better but will let you decide @jancborchardt :)

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Replace "transfer" notion with "convert" notion

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Replace "transfer" notion with "convert" notion

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Update TransferGuestDialog.vue

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Update lib/Notifications/Notifier.php

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Update lib/Notifications/Notifier.php

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Update src/components/TransferGuestDialog.vue

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Update src/components/TransferGuestDialog.vue

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Co-Authored-By: Pytal <24800714+Pytal@users.noreply.github.com>
@Jerome-Herbinet Jerome-Herbinet force-pushed the Jerome-Herbinet-transfer-to-convert branch from c1c3176 to 63f5900 Compare August 27, 2024 07:05
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.

Thanks @Pytal! :) I think it is a bit cumbersome and would be way better with both occurences of "account" deleted, so it is like this:

Conversion of guest test1@example.com to (test1) completed

Especially as the proper account has the userbubble then, it’s way easier to read.

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Copy link
Member Author

Thanks @Pytal! :) I think it is a bit cumbersome and would be way better with both occurences of "account" deleted, so it is like this:

Conversion of guest test1@example.com to (test1) completed

Especially as the proper account has the userbubble then, it’s way easier to read.

@jancborchardt check my 2 last commits.

@jancborchardt
Copy link
Member

Good stuff, thanks @Jerome-Herbinet and sorry for the back and forth :)

@Pytal Pytal merged commit 2acf585 into master Aug 29, 2024
38 checks passed
@Pytal Pytal deleted the Jerome-Herbinet-transfer-to-convert branch August 29, 2024 19:01
@Pytal
Copy link
Member

Pytal commented Aug 29, 2024

/backport to stable30

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label Aug 29, 2024
@backportbot backportbot bot removed the backport-request Pending backport by the backport-bot label Aug 29, 2024
Copy link

github-actions bot commented Sep 3, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

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 design Related to the design enhancement New feature or request feedback-requested papercut Small issues that doesn't break the ux/ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New guest transfer feature : "conversion" notion is better (IMO) than "transfer" notion + quota remains zero
3 participants