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

Integrate Nextcloud Text as editor for notes #652

Closed
wants to merge 1 commit into from
Closed

Conversation

korelstar
Copy link
Member

@korelstar korelstar commented Dec 27, 2020

Currently, this is just a proof-of-concept. See #331 for more information.

Screenshots

Before: Edit mode

Screenshot

Before: Preview mode

Screenshot

After: WYSIWYG edit mode (replaces old edit mode and old preview mode)

Screenshot

Open Tasks

regarding Notes app

  • remember last opened note
  • auto title for new notes

regarding Text app

regarding Text integration in Notes app

  • loading another note with same component (proof-of-concept uses a "hack" which does not allow for a loading indicator)
  • "Link file" feature of the Text app could be replaced by a "Link note" feature (?)
  • how to provide integrated images to 3rd-party apps using the Notes API?

@korelstar korelstar added the feature: editor Related to generic parts of the editor, e.g. loading/saving notes label Dec 27, 2020
@korelstar korelstar added this to the 5.0.0 milestone Dec 27, 2020
@stefan-niedermann
Copy link
Member

Great to see some progress here! 🥳

Will you consider implementing Context based formatting also in this PR? Because if you show the toolbar permanently and users will get used to it, it will be hard to change the behavior again in the future.

@korelstar
Copy link
Member Author

I like context based formatting and I would like to see it in this app. But with realizing the Text integration into the Notes app, there will be no code left in the Notes app regarding editor features. This means, that if the Notes app should provide context-based formatting, then this has to be implemented in the Text app. Therefore: no, this will be not part of this PR.

Nevertheless, context-based formatting could be classified as a release-critical feature. This would mean, that we won't merge this PR until Text has implemented this feature. As listed in #331 (comment), there are also other issues of this criticality. Hence, the time for merging this seems to be far away anyway.

On the other hand, I don't even know if context-based formatting is a desired feature in the Text app. This has to be discussed with the Text people and the design people. Maybe you could assess this from your perspective? @nextcloud/text @nextcloud/designers (and also /cc @nextcloud/notes )

@juliusknorr
Copy link
Member

Nice :)

Let me comment on some of the points here:

regarding Text app
don't break original file, see nextcloud/text#593 and nextcloud/text#772
use correct / other list/task syntax, see nextcloud/text#1297

I feat that we might never be able to fully preserve the original formatting. The main reason is that the WYSIWYG editor does not preserve the original format but converts the markdown document to a different document format (a json tree with different node types like h1/li and annotation types like italic/bold) and then on save converts this json format back to markdown. Taking lists as an example we just don't know when saving if the original markdown file was written with * or - as list indicators. This was decided to be an acceptable downside of the approach since we mainly aimed for a rich content editor here rather than a markdown editor. However we should of course preserve as much as possible so the open enhancement requests about frontmatter or tables for example should definitely be implemented at some point.

improve parallel changes from outside of app, see nextcloud/text#1212

Yes, we have some ideas about automatically merging the outside changes into the history of the collaboratively edited document, however this will be quite some effort to get to.

On the other hand, I don't even know if context-based formatting is a desired feature in the Text app. This has to be discussed with the Text people and the design people. Maybe you could assess this from your perspective?

I'm open to discuss this, the main personal concern here is that the formatting with a popover like shown by @jancborchardt in #200 is not really discoverable in my oppinion.

Regarding the general usage of the text app component I think your approach is fine for a first attempt. Ideally we would have the editor available as a dedicated libary that could then just be used in apps. I started in https://github.com/juliushaertl/nextcloud-text/ however didn't have much time to continue with that recently.

Also let me know if I shall add you to the text conversation on our Nextcloud Talk instance, I think you already have an account there if you want to discuss something quickly.

@juliusknorr
Copy link
Member

It might also be worth to have a look at https://gitlab.com/collectivecloud/collectives which @azul and @mejo- built last year as a prototype fund project which is based on Nextcloud Text in a similar way.

@korelstar
Copy link
Member Author

Hey Julius,

thanks for your clear statement on the WYSIWYG problematic. This helps me to look at this from the right perspective. I'm pretty sure, that some users won't like this approach, since they use the Notes app with externally generated files. But you're right, we have to weigh the pros and cons.

The most important thing IMHO is that the Notes app should still work seamlessly with the Android Notes app. My first impression was, that there will some more issues after integrating Text (besides the synchronization issue) due to the strong CommonMark based WYSIWYG approach. I will investigate that further (maybe @stefan-niedermann can also do some tests?).

Nevertheless, please add me to the Talk channel for Text, I'm already on that NC instance for other channels.

I also took a look on the collectivecloud project (accidentally) and took some code from their integration approach. That was a good starting point! :-)

@azul
Copy link

azul commented Jan 1, 2021

Hey @korelstar ... i thought that approach looked familiar 😃 - happy it was useful.

It has worked for us so far but it's really not a clean implementation. I'll be back to working more on the collectives app in march and maybe i can also take a stab at a cleaner way of using the text app in other contexts.

@inthreedee
Copy link
Member

inthreedee commented Jan 3, 2021

regarding Text app
don't break original file, see nextcloud/text#593 and nextcloud/text#772

...This was decided to be an acceptable downside of the approach since we mainly aimed for a rich content editor here rather than a markdown editor.

When deciding how to handle this situation, please see the comments in the two issue threads mentioned. In particular, nextcloud/text#772 (comment) seems to resonate. I agree that the tradeoffs as discussed by @juliushaertl are reasonable for a rich content editor but not for one that is set up as the default editor of (and then destroyer of) markdown files. For example, on iOS, it seems to be the only option now; the old edit in plain text option seems unavailable even when the new Text app is disabled.

If this new editor is not intended to perform the functions of a proper markdown editor, then it probably should not handle markdown files at all given the destructive side-effects. Give these "Nextcloud rich text format" files and notes that it creates their own file extension to avoid this situation where markdown files created in other applications get destroyed.

Basically, if it is only capable of handling markdown files in the "Nextcloud rich text format", it shouldn't be using .md files because nobody else uses the "Nextcloud rich text format" for their .md files. The best time to make this kind of change from a user's perspective would be during a large integration like this.

Edit:
On the other hand, if it is possible to handle .md file formatting properly and non-destructively then, by all means, please do that.

@vincowl
Copy link
Contributor

vincowl commented Jan 24, 2023

Hi there,
Could someone please tell us wether this feature has a chance to be implemented one day or if after a whole year without any activity, we should consider this as an implicit "forget it" answer ? I personnaly would love to use Notes, but having 2 markdown editors hosted on nextcloud (and one is the nextcloud's ref) is a strong no go for me. I would love to write my md files online or on my mobile, but would love far more something that can be cleanly integrated in the other modules (can be edited / formatted the same way). And I think this should imply choosing a default markdown parser and editor. So why not Text ?

@juliusknorr
Copy link
Member

We are going to expose the text app editor through an API, so I'll close this in favour of #958

@szaimen szaimen deleted the text branch January 31, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: editor Related to generic parts of the editor, e.g. loading/saving notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants