Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #1359: Updating U2f/WebAuthn modals to include site host #1392

Merged
merged 1 commit into from
Aug 20, 2019

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Aug 13, 2019

Fix #1359

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • My patch or PR title has a standard commit message that looks like Fix #123: This fixes the shattered coffee cup! (or No Bug: <message> if no relevant ticket)
  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New files have MPL-2.0 license header.

Test Plan:

  1. Navigate to https://demo.yubico.com/u2f
  2. Complete the verification workflow and confirm that Insert Key, Enter PIN and Touch Key modals have site URL in the title.

Screenshots:

Screen Shot 2019-08-14 at 10 10 28 AM

Screen Shot 2019-08-14 at 10 10 41 AM

Screen Shot 2019-08-14 at 10 10 52 AM

Reviewer Checklist:

  • PR is linked to an issue via Zenhub.
  • Issues are assigned to at least one epic.
  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • release-notes/(include|exclude)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable)

Copy link

@karenkliu karenkliu left a comment

Choose a reason for hiding this comment

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

Hi @jumde, thanks for your update. Please see my design feedback below (some minor revisions requested):

design QA 1
design QA 2
design QA 3

@iccub
Copy link
Contributor

iccub commented Aug 14, 2019

@karenkliu thanks for the feedback.

Regarding all these small font/spacing changes, we use a general purpose framework for these yubikey screens, which mean if we change for example spacing between title and image, it will affect all popups in our application.

Same for changing font weight, although this is easier to be modified only for these screens.

For the PIN verification image, could you provide us an image with extra empty bottom padding? So we will have to change nothing on our side, just replace the image

ccing @jamesmudgett as he's worked on the general purpose popup implementation on iOS

@karenkliu
Copy link

Hi @iccub - here is the updated image for the PIN screen. Thanks for making these updates!
PNG.zip

@jhreis jhreis merged commit 4546189 into development Aug 20, 2019
@jhreis jhreis deleted the url_modals branch August 20, 2019 18:10
@jhreis jhreis modified the milestones: 1.11.4, 1.12 Aug 20, 2019
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.

Add site url to the touch key modal
4 participants