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

feat(menu): Put list items in submenu, add indentation options #6353

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

mejo-
Copy link
Member

@mejo- mejo- commented Sep 13, 2024

Fixes: #2438
Contributes to: #2836

πŸ–ΌοΈ Screenshots

🏚️ Before 🏑 After
image image
Screencast
recording1

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests

@mejo- mejo- added bug Something isn't working 3. to review labels Sep 13, 2024
@mejo- mejo- added this to the Nextcloud 31 milestone Sep 13, 2024
@mejo- mejo- self-assigned this Sep 13, 2024
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Basically good to go, only 2 remarks:

  • A separator line between the list options and indentation options would be useful (as per the suggestion)

Sidenote: This change makes it more obvious that once you decide for ordered/unordered list or checklist, you can not change between them. Any specific reason for that? (But that's a different issue.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, actually missed something: It is "indentation", not "indention". :)

(EDIT: Actually both seem to exist, but "indentation" is the more commonly used one. If we want to be short, then we can use "Increase indent" / "Decrease indent".)

@mejo-
Copy link
Member Author

mejo- commented Sep 14, 2024

Sidenote: This change makes it more obvious that once you decide for ordered/unordered list or checklist, you can not change between them. Any specific reason for that? (But that's a different issue.

True, but that's another issue tracked in its own issue already. No idea so far, why this is the case.

@mejo- mejo- changed the title feat(menu): Put list items in submenu, add indention options feat(menu): Put list items in submenu, add indentation options Sep 14, 2024
@mejo-
Copy link
Member Author

mejo- commented Sep 14, 2024

  • A separator line between the list options and indentation options would be useful (as per the suggestion)
  • (EDIT: Actually both seem to exist, but "indentation" is the more commonly used one. If we want to be short, then we can use "Increase indent" / "Decrease indent".)

Thanks for the review @jancborchardt. Both added now, see the updated screenshot (I didn't update the screencast) 😊

@mejo- mejo- force-pushed the feat/menubar_list_submenu branch 2 times, most recently from d2ff6cf to 48d0efb Compare September 14, 2024 20:24
Makes sure they're a continuous row from 1 to 16

Signed-off-by: Jonas <[email protected]>
@mejo- mejo- merged commit 68486f8 into main Sep 16, 2024
59 of 61 checks passed
@mejo- mejo- deleted the feat/menubar_list_submenu branch September 16, 2024 18:07
@mejo-
Copy link
Member Author

mejo- commented Sep 20, 2024

/backport to stable30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
Status: β˜‘οΈ Done
Development

Successfully merging this pull request may close these issues.

Add buttons to change list indentation level
3 participants