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

pixels for bookmarks > favorites tab #3572

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Nov 13, 2024

Task/Issue URL: https://app.asana.com/0/392891325557410/1208733952836859/f
Tech Design URL:
CC:

Description:
Pixels for bookmarks > favorites tab.

Steps to test this PR:

  1. Change DailyPixel.swift so that hasBeenFiredToday returns false
  2. Add some favorites
  3. Go to the bookmarks UI and tap on the favourites tab
  4. Tap on manage -> pixel should fire m_bookmarks_ui_favorites_manage_daily
  5. Exit edit mode
  6. Edit a favorite -> pixel should fire m_bookmarks_ui_favorites_action_daily
  7. Move a favorite -> pixel should fire m_bookmarks_ui_favorites_action_daily
  8. Remove a favorite -> pixel should fire m_bookmarks_ui_favorites_action_daily
  9. Check that actions on the new tab page do not fire the above pixels
  10. Check the overlay does not trigger these pixels to fire

… tab or if the manage button on that tab is tapped
@brindy brindy requested a review from dus7 November 13, 2024 17:02
Copy link
Contributor

@dus7 dus7 left a comment

Choose a reason for hiding this comment

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

Looks good! Tested bookmarks editor and focused view. I had a crash while trying to reorder favorites when run on iOS18 and compiled with SDK 18:

Expected dequeued view to be returned to the collection view in preparation for display. When the collection view's data source is asked to provide a view for a given index path, ensure that a single view is dequeued and returned to the collection view. Avoid dequeuing views without a request from the collection view.

Fix for it is pushed to 01f3721, you can cherry pick it here or I can create a separate PR for it.

@brindy brindy merged commit 95897a7 into main Nov 14, 2024
13 checks passed
@brindy brindy deleted the brindy/add-bookmarks-ui-pixels branch November 14, 2024 10:32
samsymons added a commit that referenced this pull request Nov 14, 2024
* main:
  text zoom improvements (#3509)
  Add support for local overrides for feature flags (#3571)
  pixels for bookmarks > favorites tab (#3572)
samsymons added a commit that referenced this pull request Nov 14, 2024
# By Daniel Bernal (3) and others
# Via GitHub
* main:
  Remove the `ld_classic` build setting (#3575)
  text zoom improvements (#3509)
  Add support for local overrides for feature flags (#3571)
  pixels for bookmarks > favorites tab (#3572)
  Sync BSK with macos (#3561)
  Add possibility to stop page loading (#3553)
  [DuckPlayer] Overlay Usage Pixels (#3565)
  Update pr.yml
  Update pr.yml

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
brindy added a commit that referenced this pull request Jan 14, 2025
brindy added a commit that referenced this pull request Jan 17, 2025
Task/Issue URL:
https://app.asana.com/0/392891325557410/1208736944745857/f
Tech Design URL:
CC:

**Description**:
This reverts commit 95897a7 as the
pixels are no longer required.

**Steps to test this PR**:
1. Build and run the app.
2. Open bookmarks manager and tap on favorites view and smoke test it.
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