Skip to content

Conversation

@CDRussell
Copy link
Member

@CDRussell CDRussell commented Nov 11, 2025

Task/Issue URL: https://app.asana.com/1/137249556945/project/1208273769335188/task/1211910160515992?focus=true

Description

This is taking BookmarksBottomSheetDialog, which is generic but only ever used for a specific use case (bookmark added confirmation) and just making it specific to that use case. This simplifies it as well as better preparing it for extension to support adding an option to let users know they can enable sync.

  • BookmarksBottomSheetDialog --> BookmarkAddedConfirmationDialog

Strings moving to the correct module is done in #7127 to de-noise this one..

Steps to test this PR

Auto-dismisses

  • Visit site, tap overflow and choose Add Bookmark.
  • Verify the bookmark added dialog shows and looks the same as prod does
  • Don't interact with it; verify it auto-dismisses (after ~3.5s)
  • Choose Edit Bookmark and delete it

Edit (not as favorite)

  • Choose Add Bookmark again, don't toggle the Add to Favorites but do tap on Edit Bookmark. Verify it isn't marked as favorite.
  • Delete it

Edit (when a favorite)

  • Choose Add Bookmark again, this time toggle the Add to Favorites to enabled
  • Tap Edit Bookmark. Verify it is still marked as a favorite.

Copy link
Member Author

CDRussell commented Nov 11, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CDRussell CDRussell force-pushed the feature/craig/make_bookmark_added_dialog_less_generic branch from 2b3bf02 to 85c38e1 Compare November 14, 2025 14:57
@CDRussell CDRussell force-pushed the feature/craig/make_bookmark_added_dialog_less_generic branch from 85c38e1 to df9820c Compare November 14, 2025 15:37
@CDRussell CDRussell force-pushed the feature/craig/make_bookmark_added_dialog_less_generic branch 2 times, most recently from ca118be to 4ab4dbf Compare November 18, 2025 09:52
@CDRussell CDRussell marked this pull request as ready for review November 18, 2025 10:15
@CDRussell CDRussell requested a review from malmstein as a code owner November 18, 2025 10:15
@CDRussell CDRussell force-pushed the feature/craig/make_bookmark_added_dialog_less_generic branch from 4ab4dbf to 9158c48 Compare November 18, 2025 12:04
private val bookmarkFolder: BookmarkFolder?,
) : BottomSheetDialog(context) {

abstract class EventListener {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why not an interface?

Copy link
Member Author

@CDRussell CDRussell Nov 19, 2025

Choose a reason for hiding this comment

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

I would normally go for interface personally, but this was porting over existing code (renaming the dialog) so wanted to minimize changes made. It also seems to be fairly consistent with other dialogs/builders, so I just kept it as it was.

Screenshot 2025-11-19 at 11 13 32

@CDRussell CDRussell force-pushed the feature/craig/make_bookmark_added_dialog_less_generic branch from 9158c48 to ce58927 Compare November 19, 2025 11:16
Copy link
Member Author

CDRussell commented Nov 19, 2025

Merge activity

  • Nov 19, 11:42 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Nov 19, 11:43 AM UTC: @CDRussell merged this pull request with Graphite.

@CDRussell CDRussell merged commit b210b63 into develop Nov 19, 2025
7 checks passed
@CDRussell CDRussell deleted the feature/craig/make_bookmark_added_dialog_less_generic branch November 19, 2025 11:43
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