Add Previewer to NoteEditor
Purpose / Description
- Rename NoteEditor to NoteEditorFragment
- Introduce NoteEditor Activity which launches NoteEditorFragment into a FragmentContainerView
- Fold the intent in NoteEditorLauncher to use NoteEditor (Activity) instead of a single fragment activity
- Add previewer into a FragmentContainerView of NoteEditor with a 1 second refresh delay
- Introduce a tab layout to switch between cards generated
- Refresh Previewer when text changed, notetype changed, and note saved
- Use ResizablePaneManager to make the layout resizable
Fixes
- For GSoC 2025: Tablet & Chromebook UI
Approach
- NoteEditor is now a activity which uses NoteEditorFragment,and PreviewerFrgment in X-Large Screens
- Add previewer into a FragmentContainerView with a 1 second refresh delay
- Introduce a tab layout to switch between cards generated
- Refresh Previewer when text changed, notetype changed, and note saved
- Use ResizablePaneManager to make the layout resizable
How Has This Been Tested?
Tested on the following Android Emulators: Pixel Tablet API 36 Large Desktop API 34
Screenshots of affected screen in different themes:
-
Black Theme:
-
Plain Theme:
-
Default Light Theme:
-
Default Dark Theme:
Emulator Video: t4.webm
Checklist
Please, go through these checks before submitting the PR.
- [x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [x] You have commented your code, particularly in hard-to-understand areas
- [x] You have performed a self-review of your own code
- [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
- [ ] UI Changes: You have tested your change using the Google Accessibility Scanner
I don't have time to do a code review now, but I really like the look of it!
I don't have time to do a code review now, but I really like the look of it!
Thanks! But david gave some feedback and it was decided that some re architecting is required! So this might take a while to get merged!
Is there any way this can be split into a separate commit (or probably in a separate Pull Request), there's a lot to review here, and I worry this will slow things down getting anything moving
I'll do a separate PR.
One of the tests might still fail, I'll try to fix by tomorrow
PS: I'd recommend adding a GitHub issue, and moving most of the labels which are currently applied to this PR there
@Haz3-jolt on edit note mode, previewer icon is showing
I am not able to resize layout. @Haz3-jolt Will you check from your side once ? resize.webm
I am not able to resize layout. @Haz3-jolt Will you check from your side once ? resize.webm
Pretty sure this is a emulator caching issue? I had this once, spent hours trying to fix it, but when I created a fresh AVD it never had a repeat of the issue.
The second commit could be its own PR, very quickly merged, and used in the fragment, since similar code exists in the fragment
I feel like @david-allison remark about the fact that the fragment and the activities relies too much on the details of each other were not really taken into consideration. I'd suggest we spend some time peer coding so that I can maybe explain more what I expect here (and I hope I understand David point of view correctly so that this won't need to be done again)
Long story short: you should have an interface, maybe called
NoteEditorFragmentDelegateor something like that, that list every function that the fragment may want to call on the activity. For example it would containsnoteDidChange(NoteType nt, List<String> fields, List<String> tags), or simplynoteDidChange(Note n)or something similar, to inform the activity (if it exists) that the note did change. Then the activity will do whatever it needs to do with this information. The fragment does not need to know that the previewer will be refreshed, the fragment don't need to know anything about the previewer. The activity will implementNoteEditorFragmentDelegateandnoteDidChangewill callloadNoteEditorPreviewer.Then, insetad of a value named
noteEditorActivityyou'll get a value calleddelegate, and you'll simply calldelegate.noteDidChangewhen needed.And similarly for any other method that the fragment want to call in the activity.
The other advantage is that we can then write test for the fragment that instead of having the real activity have a mock that implements only this interface, which should make unit test quite simpler.
Reciprocally, the Fragment probably should implement an interface that contains exactly what the activity needs. And
noteEditorFragment's type should be this interface.@david-allison can you confirm that I got it right?
I've gotten the gist of it, so I think I'll try to implement this, separation of concern is important here. I'll prolly push tommorow!
Will send a ping out if I run into some roadblock/ implementation doubts, but seems pretty straightforward.
Small changes requested. Except one that would be fine as a TODO.
My main question is: did you try to really use your feature on a tablet/chromebook and check how it works while clicking everywhere to test everything? Because, there are a few things that I expect would be frustrating on real life usage. But maybe it's because I sue ankidroid a lot and so have ideas of things to test a new-ish user don't.
As an example, I can easily imagine someone wouldn't want the side pannel if they have very long text. So having a way to hide/reshow it would be nice. Intuitively, I expect that if you scroll the bar up to the side, it should close the view.
Reciprocally, if your card is very big, maybe it's nice to have a full screen view of it. I can easily imagine that I'd find it frustrating if I can only see the card in 80% of the screen, as it means I can't see what it will look like really during review time.
Also, if you edit a note with big images, the view gets back at top left, quite frustrating given that in practice, if I scrolled to some position, it's probably that I want to see this position.
If I edit a note with multiple cards, and look at card 2, it always go back to card 1. It's frustrating as I probably want to see card 2.
To be clear, it's a great PR, and I'll be okay with merge as soon as most of the nit are changed. But I'd really like if you could spend the end of GSoC on some of those features I mentioned if you don't have other ongoing thing that must be done first.
I did test it quite a bit, but yep I should have covered a lot more edge cases!
save a cloze
- the tabs aren't named correctly
The 'sample' cloze is confusing for new users and should be changed to inform them how to add a cloze
changed it!, should be pushed in a min or 2
also for tab management, for the normal previewer, it's done in TemplatePreviewerPage.kt not in TemplatePreviewerFragment so I think having it in activity would be the right choice? open to alternatives ofc
Can we make the previewer to retain state whilst typing:
* Whether the Front/Back is shown * Index of the card template (reset to 0 only if invalid)
Took a while but done!
Should be merge ready with this!
just pushed a minor fix for preview action appearing when editNote was fragmented
