Skip to content

Conversation

@kra-mo
Copy link
Member

@kra-mo kra-mo commented Aug 1, 2025

Resolves #53775

Summary

Removes:

  • “enable” for settings
  • “settings” for settings sections
  • “all” when there is no obvious subset of items
  • “show” and “open” for navigation actions
  • “changes” for applying/discarding
  • “to clipboard” when copying
  • Explaining things that cannot happen
  • Explaining things twice, right below each other
  • Unnecessary technical jargon
  • Text that sounds like marketing copy and serves no other purpose

If there is anything in particular that was there for a specific reason, please do tell me.

Checklist

@kra-mo kra-mo requested review from a team and skjnldsv as code owners August 1, 2025 15:18
@kra-mo kra-mo requested review from nfebe and susnux and removed request for a team August 1, 2025 15:18
@kra-mo
Copy link
Member Author

kra-mo commented Aug 1, 2025

cc @marcoambrosini @jancborchardt

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Looking good, let's see tests results!

@skjnldsv skjnldsv added design Design, UI, UX, etc. 3. to review Waiting for reviews feature: files papercut Annoying recurring UX issue with possibly simple fix. feature: language/translations (l10n/i18n) Localization and translation matters labels Aug 1, 2025
@skjnldsv skjnldsv added this to the Nextcloud 32 milestone Aug 1, 2025
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.

Would be nice to split such changes, at least into commits but better into separate PRs.
Because you not only change l10n strings as the commit and the PR suggests, but also remove user notifications from the actions.

So e.g. on PR with only the string change + one PR with removing notifications.

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.

I am not sure about the removed cancellation notifications, as I remember we had requests to explicitly add those to make clear whats going on.
Otherwise just a few comments.

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.

Good on removing the overbearing notifications. For the record @susnux yes we did talk about it and also had a request for it, but in practice this proves to be super annoying and results in notification fatigue and missing actually important notifications. :\

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.

Really good also on the general simpler wording @kra-mo :) makes everything much easier to read.

@kra-mo
Copy link
Member Author

kra-mo commented Aug 5, 2025

Good on removing the overbearing notifications. For the record @susnux yes we did talk about it and also had a request for it, but in practice this proves to be super annoying and results in notification fatigue and missing actually important notifications. :\

I think it is totally fair to show toasts for actions that aren't all that visible and that you might miss otherwise, but for cancellation of actions, you can see the dialog disappear. There already is visual confirmation, I don't think anyone would miss the toasts there.

@rakekniven
Copy link
Member

rakekniven commented Aug 5, 2025

Would be nice to split such changes, at least into commits but better into separate PRs. Because you not only change l10n strings as the commit and the PR suggests, but also remove user notifications from the actions.

So e.g. on PR with only the string change + one PR with removing notifications.

I fully support this.

What is the background of these changes?
It affects many apps, but only here in the server repo.

I would like us to work on the Guidelines so that all apps here are better and more consistent.

@kra-mo
Copy link
Member Author

kra-mo commented Aug 5, 2025

What is the background of these changes? It affects many apps, but only here in the server repo.

I would like us to work on the Guidelines so that all apps here are better and more consistent.

Sure, I completely agree. This is a bit of a personal initiative of mine to improve strings across Nextcloud, I started with server just to set an example and discuss initially, but I'd like to expand changes more generally.

@susnux
Copy link
Contributor

susnux commented Aug 8, 2025

Testes in cypress/e2e/files/search.cy.ts need to be adjusted

@kra-mo
Copy link
Member Author

kra-mo commented Aug 8, 2025

Testes in cypress/e2e/files/search.cy.ts need to be adjusted

Could you help me with that? I tried to look but couldn't find anything.

And the e2e one seemed to be failing for other PRs too, but maybe that was unrelated?

@kra-mo
Copy link
Member Author

kra-mo commented Aug 12, 2025

/compile rebase

This removes:
- “enable” for settings
- “settings” for settings sections
- “all” when there is no obvious subset of items
- “show” and “open” for navigation actions
- “changes” for applying/discarding
- “to clipboard” when copying
- Explaining things that cannot happen
- Explaining things twice, right below each other
- Unnecessary technical jargon
- Text that sounds like marketing copy and serves no other purpose

Signed-off-by: kramo <git@kramo.page>
@kra-mo
Copy link
Member Author

kra-mo commented Aug 12, 2025

/compile rebase

@skjnldsv skjnldsv enabled auto-merge August 12, 2025 13:24
@susnux
Copy link
Contributor

susnux commented Aug 12, 2025

@kra-mo Cypress is related because this string changed:

cy.contains('.toast-success', /Delete .* successfull/)

@kra-mo
Copy link
Member Author

kra-mo commented Aug 12, 2025

@kra-mo Cypress is related because this string changed:

cy.contains('.toast-success', /Delete .* successfull/)

Ah, thank you. Let me try and fix it.

Signed-off-by: rakekniven <2069590+rakekniven@users.noreply.github.com>
@kra-mo
Copy link
Member Author

kra-mo commented Aug 12, 2025

/compile rebase

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@skjnldsv skjnldsv disabled auto-merge August 12, 2025 15:16
@skjnldsv skjnldsv merged commit aaca29b into master Aug 12, 2025
123 of 124 checks passed
@skjnldsv skjnldsv deleted the fix/less-words branch August 12, 2025 15:16
@welcome
Copy link

welcome bot commented Aug 12, 2025

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@kra-mo
Copy link
Member Author

kra-mo commented Aug 12, 2025

Thanks everyone for the help 🥹

@skjnldsv skjnldsv mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
kra-mo added a commit that referenced this pull request Nov 7, 2025
Follow-up to #54202

Actually seeing these in production made me realize that
they look awkward without a capital letter at the start.

Signed-off-by: kramo <git@kramo.page>
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 Design, UI, UX, etc. feature: files feature: language/translations (l10n/i18n) Localization and translation matters papercut Annoying recurring UX issue with possibly simple fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adjust sharing placeholder text to make clearer what to do when there aren't any recent sharees

8 participants