From 49847ed53a8a008d2f82261c104fa79ddda15bf1 Mon Sep 17 00:00:00 2001 From: Stefan Niedermann Date: Mon, 4 Jan 2021 00:41:32 +0100 Subject: [PATCH] Fix positioning bug when toggling a checkbox --- .../notes/edit/NotePreviewFragment.java | 63 +++---------------- .../markdown/MarkwonMarkdownUtilTest.java | 24 +++++++ .../markdown/markwon/MarkwonMarkdownUtil.java | 22 ++++++- .../markwon/MarkwonMarkdownViewer.java | 24 ++----- .../markwon/span/ToggleTaskListSpan.java | 3 +- 5 files changed, 57 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java b/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java index af4a3d76..fedf3a40 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java @@ -84,67 +84,13 @@ public class NotePreviewFragment extends SearchableBaseNoteFragment implements O public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { binding = FragmentNotePreviewBinding.inflate(inflater, container, false); - binding.singleNoteContent.getMarkdownString().observe(requireActivity(), (newContent) -> { - changedText = newContent.toString(); - binding.singleNoteContent.setMarkdownString(newContent); - saveNote(null); - }); return binding.getRoot(); } @Override public void onActivityCreated(@Nullable Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); - -// MarkDownUtil.getMarkDownConfiguration(binding.singleNoteContent.getContext()) -// .setOnTodoClickCallback((view, line, lineNumber) -> { -// try { -// String[] lines = TextUtils.split(note.getContent(), "\\r?\\n"); -// /* -// * Workaround for RxMarkdown-bug: -// * When (un)checking a checkbox in a note which contains code-blocks, the "`"-characters get stripped out in the TextView and therefore the given lineNumber is wrong -// * Find number of lines starting with ``` before lineNumber -// */ -// boolean inCodefence = false; -// for (int i = 0; i < lines.length; i++) { -// if (lines[i].startsWith("```")) { -// inCodefence = !inCodefence; -// lineNumber++; -// } -// if (inCodefence && TextUtils.isEmpty(lines[i])) { -// lineNumber++; -// } -// if (i == lineNumber) { -// break; -// } -// } -// -// /* -// * Workaround for multiple RxMarkdown-bugs: -// * When (un)checking a checkbox which is in the last line, every time it gets toggled, the last character of the line gets lost. -// * When (un)checking a checkbox, every markdown gets stripped in the given line argument -// */ -// if (lines[lineNumber].startsWith(CHECKBOX_UNCHECKED_MINUS) || lines[lineNumber].startsWith(CHECKBOX_UNCHECKED_STAR)) { -// lines[lineNumber] = lines[lineNumber].replace(CHECKBOX_UNCHECKED_MINUS, CHECKBOX_CHECKED_MINUS); -// lines[lineNumber] = lines[lineNumber].replace(CHECKBOX_UNCHECKED_STAR, CHECKBOX_CHECKED_STAR); -// } else { -// lines[lineNumber] = lines[lineNumber].replace(CHECKBOX_CHECKED_MINUS, CHECKBOX_UNCHECKED_MINUS); -// lines[lineNumber] = lines[lineNumber].replace(CHECKBOX_CHECKED_STAR, CHECKBOX_UNCHECKED_STAR); -// } -// -// changedText = TextUtils.join("\n", lines); -// binding.singleNoteContent.setText(parseCompat(markdownProcessor, changedText)); -// saveNote(null); -// } catch (IndexOutOfBoundsException e) { -// Toast.makeText(getActivity(), R.string.checkbox_could_not_be_toggled, Toast.LENGTH_SHORT).show(); -// e.printStackTrace(); -// } -// return line; -// } -// ) -// .build()); - - TextProcessorChain chain = defaultTextProcessorChain(note); + final TextProcessorChain chain = defaultTextProcessorChain(note); binding.singleNoteContent.setMarkdownString(chain.apply(note.getContent())); changedText = note.getContent(); binding.singleNoteContent.setMovementMethod(LinkMovementMethod.getInstance()); @@ -152,11 +98,16 @@ public class NotePreviewFragment extends SearchableBaseNoteFragment implements O db = NotesDatabase.getInstance(requireContext()); binding.swiperefreshlayout.setOnRefreshListener(this); - SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(requireActivity().getApplicationContext()); + final SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(requireActivity().getApplicationContext()); binding.singleNoteContent.setTextSize(TypedValue.COMPLEX_UNIT_PX, getFontSizeFromPreferences(requireContext(), sp)); if (sp.getBoolean(getString(R.string.pref_key_font), false)) { binding.singleNoteContent.setTypeface(Typeface.MONOSPACE); } + + binding.singleNoteContent.getMarkdownString().observe(requireActivity(), (newContent) -> { + changedText = newContent.toString(); + saveNote(null); + }); } @Override diff --git a/markdown/src/androidTest/java/it/niedermann/android/markdown/MarkwonMarkdownUtilTest.java b/markdown/src/androidTest/java/it/niedermann/android/markdown/MarkwonMarkdownUtilTest.java index cecace60..12cb8209 100644 --- a/markdown/src/androidTest/java/it/niedermann/android/markdown/MarkwonMarkdownUtilTest.java +++ b/markdown/src/androidTest/java/it/niedermann/android/markdown/MarkwonMarkdownUtilTest.java @@ -87,6 +87,30 @@ public class MarkwonMarkdownUtilTest extends TestCase { @Test public void testLineStartsWithCheckbox() { final Map lines = new HashMap<>(); + lines.put(" - [ ] a", true); + lines.put(" - [x] a", true); + lines.put(" * [ ] a", true); + lines.put(" * [x] a", true); + lines.put(" + [ ] a", true); + lines.put(" + [x] a", true); + lines.put("- [ ] a", true); + lines.put("- [x] a", true); + lines.put("* [ ] a", true); + lines.put("* [x] a", true); + lines.put("+ [ ] a", true); + lines.put("+ [x] a", true); + lines.put(" - [ ] ", true); + lines.put(" - [x] ", true); + lines.put(" * [ ] ", true); + lines.put(" * [x] ", true); + lines.put(" + [ ] ", true); + lines.put(" + [x] ", true); + lines.put(" - [ ]", true); + lines.put(" - [x]", true); + lines.put(" * [ ]", true); + lines.put(" * [x]", true); + lines.put(" + [ ]", true); + lines.put(" + [x]", true); lines.put("- [ ] ", true); lines.put("- [x] ", true); lines.put("* [ ] ", true); diff --git a/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownUtil.java b/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownUtil.java index 33ab1978..8db791db 100644 --- a/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownUtil.java +++ b/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownUtil.java @@ -109,6 +109,25 @@ public class MarkwonMarkdownUtil { return null; } + public static CharSequence setCheckboxStatus(@NonNull String markdownString, int targetCheckboxIndex, boolean newCheckedState) { + final String[] lines = markdownString.split("\n"); + int checkboxIndex = 0; + for (int i = 0; i < lines.length; i++) { + if (lineStartsWithCheckbox(lines[i]) && lines[i].trim().length() > EListType.DASH.checkboxChecked.length()) { + if (checkboxIndex == targetCheckboxIndex) { + final int indexOfStartingBracket = lines[i].indexOf("["); + final String toggledLine = lines[i].substring(0, indexOfStartingBracket + 1) + + (newCheckedState ? 'x' : ' ') + + lines[i].substring(indexOfStartingBracket + 2); + lines[i] = toggledLine; + break; + } + checkboxIndex++; + } + } + return TextUtils.join("\n", lines); + } + public static boolean lineStartsWithCheckbox(@NonNull String line) { for (EListType listType : EListType.values()) { if (lineStartsWithCheckbox(line, listType)) { @@ -119,7 +138,8 @@ public class MarkwonMarkdownUtil { } public static boolean lineStartsWithCheckbox(@NonNull String line, @NonNull EListType listType) { - return line.startsWith(listType.checkboxUnchecked) || line.startsWith(listType.checkboxChecked); + final String trimmedLine = line.trim(); + return (trimmedLine.startsWith(listType.checkboxUnchecked) || trimmedLine.startsWith(listType.checkboxChecked)); } /** diff --git a/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java b/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java index 7ab14be2..64a551e0 100644 --- a/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java +++ b/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java @@ -46,28 +46,12 @@ public class MarkwonMarkdownViewer extends AppCompatTextView implements Markdown super(context, attrs, defStyleAttr); this.markwon = MarkwonMarkdownUtil.initMarkwonViewer(context) .usePlugin(new ToggleableTaskListPlugin((toggledCheckboxPosition, newCheckedState) -> { - // TODO move logic to MarkwonMarkdownUtil - Log.v(TAG, "new text: " + toggledCheckboxPosition); - final CharSequence unrenderedText = unrenderedText$.getValue(); - if (unrenderedText == null) { + final CharSequence oldUnrenderedText = unrenderedText$.getValue(); + if (oldUnrenderedText == null) { throw new IllegalStateException("Checkbox #" + toggledCheckboxPosition + ", but unrenderedText$ value is null."); } - final String[] lines = unrenderedText.toString().split("\n"); - int checkboxIndex = 0; - for (int i = 0; i < lines.length; i++) { - if (MarkwonMarkdownUtil.lineStartsWithCheckbox(lines[i].trim())) { - if (checkboxIndex == toggledCheckboxPosition) { - final int indexOfStartingBracket = lines[i].indexOf("["); - final String toggledLine = lines[i].substring(0, indexOfStartingBracket + 1) + - (newCheckedState ? 'x' : ' ') + - lines[i].substring(indexOfStartingBracket + 2); - lines[i] = toggledLine; - break; - } - checkboxIndex++; - } - } - this.unrenderedText$.setValue(TextUtils.join("\n", lines)); + final CharSequence newUnrenderedText = MarkwonMarkdownUtil.setCheckboxStatus(oldUnrenderedText.toString(), toggledCheckboxPosition, newCheckedState); + this.setMarkdownString(newUnrenderedText); })) .build(); this.renderService = Executors.newSingleThreadExecutor(); diff --git a/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/ToggleTaskListSpan.java b/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/ToggleTaskListSpan.java index 1c1dea01..6b7c856a 100644 --- a/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/ToggleTaskListSpan.java +++ b/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/ToggleTaskListSpan.java @@ -11,7 +11,6 @@ import androidx.annotation.NonNull; import java.util.Arrays; import java.util.function.BiConsumer; -import java.util.function.Consumer; import io.noties.markwon.ext.tasklist.TaskListSpan; @@ -33,7 +32,7 @@ public class ToggleTaskListSpan extends ClickableSpan { public void onClick(@NonNull View widget) { span.setDone(!span.isDone()); widget.invalidate(); - Log.i(TAG, "task-list click, isDone: " + span.isDone() + ", content: '" + content + "'"); + Log.v(TAG, "task-list click, isDone: " + span.isDone() + ", content: '" + content + "'"); // it must be a TextView final TextView textView = (TextView) widget;