Move overflow menu into bottom modal sheet. #147
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move piefed comment notification controls into overflow menu.
…ps I'll just sneak this in here :).
jwr1
requested changes
Apr 7, 2025
Member
jwr1
left a comment
jwr1
left a comment
There was a problem hiding this comment.
Thank you, this looks great! Just two things I've noticed:
- Clicking on the various actions (e.g., upvoting, bookmarking, etc.) does not update the UI in the popup modal (upvoting does not change the number or arrow color). There are two ways to solve this: either update the state, or close the popup when an action is performed. Closing the popup once an action is performed would match it up with the way it worked previously, so that might be the best way to go.
- Again, I know this means more work, but could you update ContentItemCompactPost, as that's where this sort of popup would really be helpful (as there are no other controls).
Move post item inkwell into post item widget. Add longpress call back to post item widget which shows the content menu.
Contributor
Author
|
Since they share a lot of functionality I moved the compact content item into a function in the full content item and which widget structure is returned in build is chosen based on isCompact. This should be easier to keep action callbacks consistent between them with just the ui changing. |
jwr1
requested changes
Apr 8, 2025
Member
jwr1
left a comment
jwr1
left a comment
There was a problem hiding this comment.
Combining the two content item's into the same class is a good call. I just have three last things:
- Now that you can long tap over the post to pop up the menu, can you make it so right clicking does the same (long tap and right click are paired in several other places as well)?
- Opening the full post view still has the ink well there, even though tapping on it doesn't do anything. So, probably make the long tap (and right click) only work if the post is in the feed (not full screen).
- The ban user dialog no longer works, due to
Navigator.popbeing used immediately after and closing the dialog. Movingpopto before running the ban action should fix it.
Disable longpress/rightclick post item when not on feed screen. Pop the content menu before moderateBan callback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move overflow menu into bottom modal sheet.
Also added notification controls and voting controls with separate upvotes/downvote counts to top of new modal menu.
Closes #140.