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

Changes menu entry. #2292

Merged
merged 1 commit into from
Apr 27, 2022
Merged

Changes menu entry. #2292

merged 1 commit into from
Apr 27, 2022

Conversation

pierreozoux
Copy link
Member

@pierreozoux pierreozoux commented Apr 12, 2022

We realized with our users that when the apps Text and ONLYOFFICE are installed in parallel, this is a bit confusing.

You can see a screenshot here, in french:
https://forge.liiib.re/indiehost/tech/plateforme/-/issues/217#note_9733

So we propose this change.

In relation to this change in onlyoffice app.

@max-nextcloud
Copy link
Collaborator

Hi @pierreozoux

Thanks a lot for your proposal. I'm a bit unsure about using 'Note' instead as there is a Notes app - so this might be confusing if you create a note somewhere and then it does not show in the notes app (because that app only works on one folder).

I agree that New text document is confusing. For me it's mostly the document part. That sounds like something formal on a page with layout and all that.

My personal preference would be just New text - do you think that would make it easier to distinguish?

I'd be really curious what @jancborchardt and @nimishavijay think.

@pierreozoux
Copy link
Member Author

@ImaCrea what do you think?

@ImaCrea
Copy link

ImaCrea commented Apr 15, 2022

@ImaCrea what do you think?

What about New pad @max-nextcloud ?

@vinicius73 vinicius73 requested a review from juliusknorr April 18, 2022 11:23
@juliusknorr
Copy link
Member

The text app is advertised and documented as a text editor so from my perspective either "New text document" or "New text file" are the most fitting terms. Pad is not really a term that is understandable for non-technical users from my perspective and note is something that may add a wrong connection to the notes app, but I would appreciate feedback from @jancborchardt and @nimishavijay here as well :)

@ImaCrea
Copy link

ImaCrea commented Apr 19, 2022

The text app is advertised and documented as a text editor so from my perspective either "New text document" or "New text file" are the most fitting terms. Pad is not really a term that is understandable for non-technical users from my perspective and note is something that may add a wrong connection to the notes app, but I would appreciate feedback from @jancborchardt and @nimishavijay here as well :)

Understood. New text file sounds good to me then. It's the 'document' part of the current wording that can be misleading when office suite like collabora or onlyoffice are integrated.

@nimishavijay
Copy link
Member

I can understand how it could be confusing when there is a "New document" right underneath.
"New text" seems a bit incomplete, and "New pad" won't be understood by many as I've seen it being used only with EtherPad, so I would vote for "New text file" :)

@jancborchardt
Copy link
Member

Yep, agree with the points made by @max-nextcloud @juliushaertl and @nimishavijay. Let’s go for "New text file". :)

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

See inline comment for the wording as per discssion, in addition the wording in

$markdownFile = new TemplateFileCreator(Application::APP_NAME, $l->t('New text document'), '.md');
will also need to be adjusted.

src/helpers/files.js Outdated Show resolved Hide resolved
@juliusknorr juliusknorr added enhancement New feature or request 2. developing labels Apr 20, 2022
@blizzz blizzz added this to the Nextcloud 25 milestone Apr 21, 2022
@juliusknorr
Copy link
Member

Looks good for the frontend part, could you also adjust that accordingly in

$markdownFile = new TemplateFileCreator(Application::APP_NAME, $l->t('New text document'), '.md');
?

@pierreozoux pierreozoux force-pushed the pierreozoux-patch-1 branch 2 times, most recently from 2938eb2 to 3ba29a5 Compare April 26, 2022 13:32
@pierreozoux
Copy link
Member Author

@juliushaertl I think I did all the changes, looks goot to 🚢 to me :)

@max-nextcloud
Copy link
Collaborator

/compile amend /

@max-nextcloud
Copy link
Collaborator

/rebase

@max-nextcloud
Copy link
Collaborator

I rebased on top of the current master and rebuild the js. Hope that tests pass now and we can merge after.
Thanks a lot for your contribution @pierreozoux

We realized with our users that when the apps Text and ONLYOFFICE are installed in parallel, this is a bit confusing.

You can see a screenshot here, in french:
https://forge.liiib.re/indiehost/tech/plateforme/-/issues/217#note_9733

So we propose this change.

In relation to this change in [onlyoffice app](ONLYOFFICE/onlyoffice-nextcloud#617).

Signed-off-by: Pierre Ozoux <pierre@ozoux.net>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@max-nextcloud
Copy link
Collaborator

cypress failure is not related.Will look into that separately.

@max-nextcloud max-nextcloud merged commit 2091aec into master Apr 27, 2022
@delete-merged-branch delete-merged-branch bot deleted the pierreozoux-patch-1 branch April 27, 2022 11:44
@pierreozoux
Copy link
Member Author

@max-nextcloud do you think it is possible to backport this little change? Or is it a "big" change, and will have to wait 25th?

@max-nextcloud
Copy link
Collaborator

I'd be okay with backporting this - I'll open a backport pr to have the discussion there.

@max-nextcloud
Copy link
Collaborator

/backport to stable24

@pierreozoux
Copy link
Member Author

/backport to stable23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing backported successfully backported enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants