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

Improve post/comment bottom sheets #1199

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Mar 12, 2024

Pull Request Description

This PR aims to improve the ever-expanding list of actions for posts and comments by grouping some actions.

  1. User-, community-, and instance-related actions are now grouped in respective sub-menus.
  2. The options now directly reference the entities by name.
  3. The bottom sheet now supports the Android back button to navigate back from sub-menus.
  4. For own posts, delete is now moved up to the top row with the other post-related actions.

These changes required a bit of refactoring so that the architecture matches that of other menus like the modlog one. So this is one of those cases where testing may be more fruitful than reviewing. 😊 This refactor should help with future maintainability and usability.

Here are some things that I identified that didn't quite work right before (so this PR doesn't break them), but they should be fixed in the future (and some of them will now be easier!):

  • Remove ability to block self
  • Add ability to unblock users/communities (i.e., check current block state)
  • Immediately update subscribe state rather than having to refresh feed
  • Move comment "Copy Text" to a sub-menu and also provide the option to make the text selectable and show raw (#1127)
  • Improve the share sub-menu for both posts and comments to provide options for the current instance and the original instance (#1047)

Review without whitespace.

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Posts

qemu-system-x86_64_pMCRnkTt2z.mp4

Comments

qemu-system-x86_64_bGIJzWk8Ty.mp4

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

@hjiangsu
Copy link
Member

hjiangsu commented Mar 14, 2024

The demo videos look great! Thanks for doing this, it was a much needed change 😅

I do have one suggestion for the user/community/instance (the main bottom sheet) - do you think its possible to move the entity names to a subheading within the options?

  • This allows more horizontal space for the entity name and would look cleaner in my opinion
  • PickerItem has a subtitle parameter that we could use for this (I added it in for the full date format)!

So this is one of those cases where testing may be more fruitful than reviewing

I agree! I just took a very brief look at the code changes.

@micahmo
Copy link
Member Author

micahmo commented Mar 14, 2024

do you think its possible to move the entity names to a subheading within the options

Great idea! I just pushed this, how does it look?

Header Header
image image

@hjiangsu
Copy link
Member

Perfect! I'll go ahead and merge this in so that we can get some early testing done

@hjiangsu hjiangsu merged commit e5ab0e3 into thunder-app:develop Mar 14, 2024
1 check passed
@micahmo micahmo deleted the feature/improve-bottom-sheets branch March 14, 2024 17:17
@micahmo micahmo mentioned this pull request Mar 24, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants