Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Conversation

@mallexxx
Copy link
Collaborator

@mallexxx mallexxx commented Aug 31, 2023

Task/Issue URL: https://app.asana.com/0/1177771139624306/1202217488822824/f
Figma: https://www.figma.com/file/pJbEb2nC4jwXlToaV0xwGD/Time-boxed-UI-polish-round-(2022-09)?type=design&node-id=48-15266&mode=design&viewport=-2903%2C2125%2C1.15&t=VC4yzgdLvv3RaUhD-0

Description:

  • Align popovers in bounds of the Main Window
  • Align Bookmark popover to the left, Privacy Dashboard to the right
  • Align More (...) by the right edge
  • Made Fire Button close on second button click and appear on mouse-down
  • disabled animations for the rest of popovers (as of Bookmarks/Autofill/NetP popovers not animated)

!! it seems the approach doesn‘t work in latest Sonoma beta: https://app.asana.com/0/0/1205377179358269/1205391501334690/f - I‘m downloading it to inspect

Steps to test this PR:

  1. Validate popovers are positioned in bounds of the Main Window and still respect screen bounds and work in Full Screen
  2. Validate More Menu positioning
  3. Validate Fire button popover appearing/disappearing on button click and aligned in the middle of the button (I did the best I could for it)

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

Just a couple of comments. If there's nothing you can do about the fire button popover in full screen then will have to see it'll pass the ship review.

private func setupFireButton() {
fireButton.toolTip = UserText.clearBrowsingHistoryTooltip
fireButton.animationNames = MouseOverAnimationButton.AnimationNames(aqua: "flame-mouse-over", dark: "dark-flame-mouse-over")
fireButton.sendAction(on: .leftMouseDown)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this doing? Didn't seem to have any affect when I commented it out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it‘s making the popover appear on mouse-down, not on click, it‘s used for other popovers too

@mallexxx mallexxx requested a review from brindy September 1, 2023 17:15
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

Just a print statement to remove, but otherwise LGTM

@mallexxx mallexxx merged commit 932b58b into develop Sep 4, 2023
@mallexxx mallexxx deleted the alex/popover-positioning branch September 4, 2023 09:47
samsymons added a commit that referenced this pull request Sep 4, 2023
# By Alexey Martemyanov (5) and others
# Via GitHub (1) and Tomas Strba (1)
* develop:
  DBP Debug UI (#1587)
  Bump version to 1.55.0 (57)
  Update embedded files
  Auto start DBP scheduler (#1586)
  Add broker updater mechanism (#1580)
  Update develop with Bitwarden fix (#1585)
  fix outlook popups loading; set popup window mode to match Safari (#1564)
  remove AppActivityMonitor and related unused code (#1582)
  Popovers positioning (#1574)
  Version 1.54.1
  xcode 15 synthesized color constants fixes (#1583)
  Fixes issue with password saving when using Bitwarden (#1584)
  Remove the incremental test pixel sender (#1578)
  Implement waitlistBetaActive logic (without actually resetting) (#1562)
  Lock Form fill when autofill is locked (#1559)
  show alert when user tries to open file in AppStore version (#1494)
  DBP UI Middleware  (#1553)
  Dashboard & Profile Creation  (#1567)
  roll out the bookmarks bar experiment (#1572)
  Open duck player for youtube same-document navigation (#1560)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/Common/Localizables/UserText.swift
#	DuckDuckGo/NavigationBar/PinningManager.swift
#	DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift
#	DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift
#	UnitTests/Menus/MoreOptionsMenuTests.swift
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants