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 May 3, 2023

Task/Issue URL: https://app.asana.com/0/42792087274227/1204180319229124/f

Description:
Adds "QR Code" item to Share menu

Steps to test this PR:

  1. Validate QR code sharing works (regular QR code) both for regular and ddg URLs
  2. Validate QR code can be scanned for very long URLs (large QR code)

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

This looks and works really great! I’ve tested long URLs and punycoded ones and proper QR codes are generated every time. Please update Dax logo and consider parametrizing scale, as per my comments. Thanks @mallexxx!

}

var ciImage: CIImage {
var rect = NSRect(x: 0, y: 0, width: self.size.width * 2, height: self.size.height * 2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the * 2 here to handle Retina? Can we try using NSScreen.main?.backingScaleFactor and fall back to 2?

Same applies to CIImage.bitmapRepresentation(using format: NSBitmapImageRep.FileType).


static let duckDuckGo: QRCodeParameters = {
let icon: CIImage = {
let logo = NSImage(named: "Logo")!
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is using the old Dax icon. Please replace with the new one.

@ayoy ayoy assigned mallexxx and unassigned ayoy May 12, 2023
@mallexxx mallexxx merged commit 1f5e1f6 into develop May 12, 2023
@mallexxx mallexxx deleted the hackdays/alex/qr-share branch May 12, 2023 18:04
samsymons added a commit that referenced this pull request May 23, 2023
* upstream_develop:
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  Update metadata
  Fix creating DMG from release workflow (#1200)
  Unit tests to cover sensitive areas of Burner Window (#1169)
  [Hackdays] Share via QR Code (#1177)
  Fix sync connect flow  (#1195)
  Update to BSK adding new remote message image (#1194)
  Set version to 1.39.0
  Update embedded files
  Update old Dax asset (#1193)
  WebKit find in page (#1188)
  modify manage bookmarks button (#1175)
  sync update device model / poll for devices (#1191)
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