Conversation
Update mise a jour
|
@vbernier02 everything okay? :) |
Yes, just a little fix with the import |
|
I don't know what to do about NotesDatabase.kt:90:59: This expression contains a magic number. Consider defining it as a well-named constant. [MagicNumber]. |
|
Hey, don't worry about it for now. Some checks are failing on master as well. I'll be soon updating the contribution guidelines regarding this. |
naveensingh
left a comment
There was a problem hiding this comment.
I found some issues:
- The read-only state is still lost on app restart. I think we can remove persistence for now and keep it session-based. It'll be a simpler implementation and probably less confusing for most users.
- Spell check should be disabled in read-only mode. Right now, I can modify the note by tapping highlighted words or phrases (check out this)
- The keyboard should be hidden automatically when read-only mode is activated (use
hideKeyboardextension) - The undo button now appears automatically on startup, even when there are no edits.
Ok, I'll check that myself later during review. You may focus on points 2, 3, and 4
This is the expected behavior. Icons in buttons represent the action that will be performed. Not the current state. (Check this app for example)
Yes, ignore |
|
I think this should fix the spell check and keyboard issue. I haven't looked at the undo button yet. |
Yep, those seem to be fixed now 🎉 |
|
But it looks like your latest commit broke selection in read-only mode. Now I also get a crash on long press: Process: org.fossify.notes, PID: 31839
java.lang.NullPointerException: Attempt to invoke virtual method 'int android.widget.Editor$SelectionModifierCursorController.getMinTouchOffset()' on a null object reference
at android.widget.Editor.touchPositionIsInSelection(Editor.java:1386)
at android.widget.Editor.performLongClick(Editor.java:1477)
at android.widget.TextView.performLongClick(TextView.java:13025)
at android.view.View.performLongClick(View.java:7656)
at android.view.View$CheckForLongPress.run(View.java:30144)
at android.os.Handler.handleCallback(Handler.java:942)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loopOnce(Looper.java:211)
at android.os.Looper.loop(Looper.java:300)
at android.app.ActivityThread.main(ActivityThread.java:8503)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:561)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:954) |
I haven't been able to reproduce the crash but I think it has to do with the fact that I deleted some lines which during my last tests no longer seem to influence the code. So I think I was wrong. |
Yes, that was it. No crashes now. |
|
Nice, thank and sorry for making you test it once more. |
It's usually not the build, and some issues only manifest under specific conditions, but here are some options to get a clean slate:
|
|
This pull request was automatically closed due to inactivity and/or lack of response from the author. If you have further updates or additional information, please comment or reopen the PR to continue. |
|
Hey, still working on this? I'm happy to take it from here if you aren't free. |
Sorry, I've been pretty busy lately and probably will be for a while longer. |
|
Okay. Thanks for working on this! |

What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
after_read_only.mp4
Fixes the following issue(s)
Acknowledgement