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

Render markdown inline #2

Closed
jancborchardt opened this issue Oct 5, 2015 · 6 comments
Closed

Render markdown inline #2

jancborchardt opened this issue Oct 5, 2015 · 6 comments
Milestone

Comments

@jancborchardt
Copy link
Member

Just like the Notes web app, Markdown should render inline. Not with a dedicated view, but like iA Writer does with the special characters still showing (although subdued in grey).

cc @Henni who implemented this for the web app side.

stefan-niedermann pushed a commit that referenced this issue Jan 21, 2016
stefan-niedermann pushed a commit that referenced this issue Mar 4, 2016
@korelstar
Copy link
Member

I've found an interesting library which supports inline rendering for EditText fields in Android: https://github.com/yydcdut/RxMarkdown

However, I didn't test it yet, so everybody is welcome to evaluate if the library fits our demands. It seems to be a rather new project.

stefan-niedermann added a commit that referenced this issue Dec 7, 2016
Replace Bypass library with RxMarkdown library
@stefan-niedermann
Copy link
Member

Great lib @korelstar! You may want to check out the branch RxMarkdown?
I switched from Bypass to RxMarkdown there but i am still facing two problems:

  1. (I think its caused by the library) If you edit a note and switch to "preview" mode (eye icon), then select a word and touch anywhere instead of the back button and instead of an action (like copy or cut) -> app crashes
  2. Not 100% reproducable for me, but going back from NoteEditView to the list, the app sometimes crashes
 Caused by: java.lang.IndexOutOfBoundsException: Invalid index 0, size is 0
  at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:255)
  at java.util.ArrayList.get(ArrayList.java:308)
  at it.niedermann.owncloud.notes.model.ItemAdapter.getItem(ItemAdapter.java:132)
  at it.niedermann.owncloud.notes.android.activity.NotesListViewActivity.onActivityResult(NotesListViewActivity.java:394)

rxmarkdown

@korelstar
Copy link
Member

Yeah 🎉

In general it looks good. But I have some remarks on this:

  • I didn't made a performance test on an old device, yet. But we don't want to re-introduce the old issue Performance on opening a note #39, hmm?
  • I see some (design) issues in the layout of the preview (e.g. margin/padding to the left are bad for headings and the gray vertical line for block quotes). Do you see them, too (please compare with my screenshot below)?
  • In the EditView it would be nice, if URLs are automatically translated to links, too.
  • ad your problem 1.: I've observed this error already with the old library. However, it only occurs in the emulator and not on my mobile phone. Hence, I didn't investigated it further. Does this happen on your physical device?
  • ad your problem 2.: This could be due to my changes (8384792) in the NotesListViewActivity: now, the list of notes is loaded in the background (RefreshListTask) in order to keep the app responsive. If the instance of NotesListViewActivity is destroyed due to a huge foreground activity (e.g. edit/preview note activity with markdown rendering), then it's possible that NotesListViewActivity.onActivityResult finds an empty notes list (since the RefreshListTask is not yet done) and the error is thrown. I will make a pull request to fix this.

editview textview

@stefan-niedermann
Copy link
Member

stefan-niedermann commented Dec 10, 2016

I didn't made a performance test on an old device, yet. But we don't want to re-introduce the old issue #39, hmm?

I did neither. Could maybe be an issue, but i have no old device here to test, only virtual machines where everything works fluid. @korelstar are you able to test on a weak or old device?

I see some (design) issues in the layout of the preview (e.g. margin/padding to the left are bad for headings and the gray vertical line for block quotes). Do you see them, too (please compare with my screenshot below)?

I experience these issues, too. The gray vertical line for block quotes only hurts if you have a single very long line of text. Nevertheless i opened an issue at RxMarkdown (and one for further purposes 😉 ). I agree this is suboptimal. Do you or @jancborchardt think this is a issue that stops the rollout of this new feature?

In the EditView it would be nice, if URLs are automatically translated to links, too.

☑️ Looks like hyperlinks, images and checkboxes may be supported in the future. But this is no issue at all since the current implementation does not enable hyperlinks in the edit-mode, too.

ad your problem 1.: I've observed this error already with the old library. However, it only occurs in the emulator and not on my mobile phone. Hence, I didn't investigated it further. Does this happen on your physical device?

☑️ Tested on my physical device and confirmed that this behavior only exists on virtual machine.

ad your problem 2.: This could be due to my changes (8384792) in the NotesListViewActivity: now, the list of notes is loaded in the background (RefreshListTask) in order to keep the app responsive. If the instance of NotesListViewActivity is destroyed due to a huge foreground activity (e.g. edit/preview note activity with markdown rendering), then it's possible that NotesListViewActivity.onActivityResult finds an empty notes list (since the RefreshListTask is not yet done) and the error is thrown. I will make a pull request to fix this.

☑️ Yeap, already merged the pull request into the RxMarkdown-branch, thanks for that.

@korelstar
Copy link
Member

I've tested it on my Samsung Galaxy Gio (from 2011). This device has a very slow memory card (in particular, writes are very slow). I've compared the Notes app in three variants: a) using RxMarkdown, b) using bypass, c) with no markdown-support (using the bypass version and remove all respective code fragments).

Interestingly, I can't see any notably difference.

  • The start of the app is in all versions rather slow (about 4 seconds until the list of notes is shown).
    • This could be due to the fact, that always the full note is loaded (even if the text length is very large).
    • But the time, until the empty activity is shown, is also rather long. This could be also due to the fact, that many support libraries are used. My grocery list app which don't uses any support libraries starts considerably faster.
  • The start of the edit and preview activity is quite fast in all cases. So I wasn't able to reproduce the old issue Performance on opening a note #39 at all. Maybe, it depends on the Android version, when the libraries are loaded. But I don't know.

So I think, using RxMarkdown will not slow down the app experience. However, I see some potential for optimization on slower devices (I've created a new issue for that: #154).

stefan-niedermann added a commit that referenced this issue Dec 11, 2016
- better font-sizes for h1-h6
- enable RxMarkdown on creating notes (not only editing)
@stefan-niedermann stefan-niedermann added this to the 0.10.0 milestone Dec 14, 2016
@stefan-niedermann
Copy link
Member

stefan-niedermann commented Jan 20, 2017

Feature is in master-branch and will be released soon. I am very hopeful that the new RxMarkdown lib is better maintained than bypass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants