Skip to content

"Share with" wording fix in new share panel #41155

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.

Sign up for GitHub

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

Closed
Jerome-Herbinet wants to merge 3 commits into from
Closed

"Share with" wording fix in new share panel #41155

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4702f0b
Update SharingDetailsTab.vue
Jerome-Herbinet Oct 27, 2023
b9bc4df
npm run build
Jerome-Herbinet Oct 27, 2023
7dee136
fixup!: Ensure that the string can be translated independed of senten…
susnux Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Update SharingDetailsTab.vue 
Signed-off-by: Jérôme Herbinet <[email protected]>
  • Loading branch information
@Jerome-Herbinet
Jerome-Herbinet authored Oct 27, 2023
commit 4702f0be57763886f3f6061d3c4b2dcdb6ddabad
2 changes: 1 addition & 1 deletion apps/files_sharing/src/views/SharingDetailsTab.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export default {

computed: {
title() {
let title = t('files_sharing', 'Share with ')
let title = t('files_sharing', 'Shared with') + ' ';
Copy link
Contributor

@susnux susnux Oct 27, 2023

Choose a reason for hiding this comment

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

Would it not make more sense to have full separate strings for every option?
I can imagine there are lanuages that do not work lik "share with" + " " + "option".
E.g. Japanese does not work like that, so it will be pretty hard to translate.

Copy link
Member Author

@Jerome-Herbinet Jerome-Herbinet Oct 27, 2023

Choose a reason for hiding this comment

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

It makes sense. Let's wait for other reviewers' opinion.

Copy link
Contributor

@susnux susnux Oct 30, 2023

Choose a reason for hiding this comment

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

cc @rakekniven

Copy link
Member

@rakekniven rakekniven Oct 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@susnux See my comment below. A screenshot would help a lot.

Copy link
Contributor

@nfebe nfebe Oct 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Fine with taking out the space.

As for the word Shared it would have to be dynamic, because that would not be true for new shares.

Copy link
Member Author

@Jerome-Herbinet Jerome-Herbinet Oct 30, 2023

Choose a reason for hiding this comment

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

You are right but in this case, I won't have the required skills to do such changes ; my skills scope is CSS design and wordings calls change.

Copy link
Member

@jancborchardt jancborchardt Nov 2, 2023

Choose a reason for hiding this comment

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

@fenn-cs could you adjust it so it works in both cases? :)

if (this.share.type === this.SHARE_TYPES.SHARE_TYPE_USER) {
title = title + this.share.shareWithDisplayName
} else if (this.share.type === this.SHARE_TYPES.SHARE_TYPE_LINK) {
Expand Down