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

feat(editor): Use text editor #958

Merged
merged 6 commits into from
Mar 20, 2023
Merged

feat(editor): Use text editor #958

merged 6 commits into from
Mar 20, 2023

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Jan 30, 2023

  • 🐛 Check issues with sidebar loading when using the new editor
  • Add option to switch to the new editor
  • Make it default for new installations
    • Ensure .md is the default file format
    • Ensure that the text editor is the default but fallback to easymde if text is unavailable
  • Migration process (if notes was installed before)
    • Show information about new editor and suggest to switch to it

From the old pull request

List of issues this addresses/resolves

@newhinton
Copy link
Contributor

how to provide integrated images to 3rd-party apps using the Notes API?

this is already implemented, see here: #785

@newhinton
Copy link
Contributor

Ensure .md is the default file format

We probably should think very careful about that one. Otherwise we have the same issue as in nextcloud/text#593.

Why dont we do the following:

  • The user did not change the default file-ending:

    • Use .ncdata if nextcloud/text is used
    • Use .md/.txt if easymde is used (depending on if we want to stick to the current default or change it)
  • The user did change the default file-ending:

    • Use what the user supplied

This way we dont get into that whole discussion again, while keeping it relatively simple.

The UX would be that the dropdown for the editor would also change the displayed value for the file-ending. That would not reqire any additional ui-elements.

The additional benefit is that those files would also be editable without the notes app, directly from the files app.

@juliushaertl juliushaertl force-pushed the enh/text branch 4 times, most recently from c57ab21 to cac5a12 Compare March 17, 2023 10:09
@juliushaertl juliushaertl marked this pull request as ready for review March 17, 2023 10:14
@juliushaertl juliushaertl added the enhancement New feature or request label Mar 17, 2023
@szaimen
Copy link
Contributor

szaimen commented Mar 17, 2023

Is master not protected?

@juliushaertl
Copy link
Member Author

Right, added that.

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Didn't read the code yet.

  1. I face the conflict view after editing in "rich text mode", not waiting for the top-right checkmark to say it was saved, switching to "edit mode" and making a change.
  2. After a change in "edit mode", even after the document has been saved (asterisk disappears in the navigation menu), switching to "rich text mode" also brings up the conflict view.

Maybe those issues are not so important as few people will switch between display modes...

Otherwise works like a charm.

Copy link

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

I had a look at the code only. Looks good to me.

@juliushaertl
Copy link
Member Author

Let's handle the conflict when switching separately. We need some extra handling for ensuring that the file is saved before switching views then.

@juliushaertl juliushaertl merged commit fdd123c into master Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paste/dragdrop images Integrate Nextcloud Text
5 participants