-
Notifications
You must be signed in to change notification settings - Fork 121
Improve nested lists behaviour #979
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
Conversation
AmandaRiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@planarvoid Changes look good and list manipulation definitely seems better, but I did run into funkiness with the enabling/disabling of the indent/outdent buttons:
list-button-enable-disable.mp4
As well as with highlighting list items across indentations and attempting to indent further:
list-highlight.mp4
And some items losing their list status:
list-issue-2.mp4
|
Thanks for the review @AmandaRiu, all items were great catches! I have fixed all of them in the last 3 commits and I think everything works well |
…matter.kt Co-authored-by: AmandaRiu <5810477+AmandaRiu@users.noreply.github.com>
Co-authored-by: AmandaRiu <5810477+AmandaRiu@users.noreply.github.com>
AmandaRiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What an incredible improvement @planarvoid ! I tested a bunch and only found one issue, a scenario that leads to an ANR. I was able to replicate the ANR with the following steps:
- Create an ordered list that looks like this:
- place cursor in "Four" and tap the button to outdent that item so the three middle list items are all at the same indentation.
- Select items "three", "four" and "five" and tap the button to outdent them so that all list items will be in the same level. This causes the app to freeze and eventually an ANR will display.
Here's a video:
screencapture-1652235250473.mp4
…other-items Implement indent and outdent across multiple item types
|
Good catch @AmandaRiu ! It was caused by a solution I've used previously to determine the following list item (to find out the desired behaviour of the outdent). I've fixed it in 8f1f87c |
AmandaRiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works beautifully!!! Very nice work @planarvoid !!! 🥳
![]()

Fix
The nested lists were not previously working properly. In this PR I'm fixing the behaviour.
Test
toolbar.setToolbarItems(ToolbarItems.BasicLayout(ToolbarAction.LIST, ToolbarAction.INDENT, ToolbarAction.OUTDENT))in theMainActivityto enable indents and list operationsindentlisticon is highlighted according to the selected listReview
@AmandaRiu
Make sure strings will be translated:
strings.xmlas a part of the integration PR.improved-lists.mp4