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

TUP-517 and TUP-472: MFA pairing UI/accessibility tweaks #265

Merged
merged 7 commits into from
Aug 2, 2023

Conversation

jarosenb
Copy link
Collaborator

@jarosenb jarosenb commented Jul 12, 2023

Overview

  • Add "MFA Documentation" link to the MFA Pairing section of the Manage Account page.
  • Add a link to the TOTP Pairing page that goes to the relevant section of the MFA docs.
  • Show the text code after generating a TOTP key, so that users without cameras can pair (TUP-472).
  • Add text directing users to submit a ticket if they don't receive a text message after requesting SMS pairing.

Related

Changes

Testing

  1. In dev, try pairing and unpairing using both the TOTP and SMS flows.

UI

image image

image

Notes

@wesleyboar
Copy link
Member

wesleyboar commented Jul 12, 2023

UI review+design working meeting (as TUP-535) scheduled for Friday morning.

@wesleyboar
Copy link
Member

wesleyboar commented Jul 18, 2023

TUP-535 comment describes designer changes. TUP-535 has new attached designs.

I am available to make the changes after I empty my plate.

@wesleyboar
Copy link
Member

Design changes (and markup and accessibility tweaks) are ready for review:

* refactor: move link to to docs (TUP-535:4,5)

* refactor!: prepare for c-nav--piped (TUP-535:3)

The pipe i slost, but will return when core-styles.base.css is v2.12.1.

Currently, this requires TACC/Core-CMS to load TACC/Core-Styles v2.12.1.

Maybe (later), Portal should use diff version than CMS.

BREAKING CHANGE: This removes the pipe.

* refactor: change 5m msg, get help link (TUP-535:6)

- Change text of the "5 minutes" message.
- Add either link to trigger modal open or link to open modal.*

* This prevents two modals being hidden into the makup.

* refactor: change qr error to reuse a help modal

* style: npx nx format:write

* refactor: simplify `qr-code-box` markup and styles

* refactor: use lists for mfa panels

* fix: missing space before "Get help." link

* refactor: simpler messages (TUP-535:2.1.1+2.1.2)

- use one tag and one class
- remove extra markup
- refactor panel layout to support new message position

* refactor: let modal manage duplicate instances

- remove conditional modal opening code
- do not say "please" in messages

* fix: restore code that I had made text for testing

* chore: remove now-unnecessary id form markup

* feat: new message if QR code alt. is unavilable

* fix: add periods to end of sentences…

* chore: remove now-unnecessary id from props

* chore: remove now-unnecessary id from props

* fix: load v2.13 c-nav component

* chore: improve a comment

* style: npx nx format:write

* fix: bad grammar in error message

* fix: bad grammar, remove please

* feat: TextCopyField.jsx to .tsx, and enable it

* feat: TextCopyModal

* fix: FieldWrapper, opt. req. & let desc. be React

* fix: FieldWrapperFormik, let desc. be React

* feat: text copy modal (TUP-535:2.1.3)

* chore: uninstall react-copy-to-clipboard

* feat: install @tacc/core-styles 2.14.0

* refactor: FieldWrapper mirror FeildWrapperFormik

Make FieldWrapper look like FeildWrapperFormik.

Lets FieldWrapper be used independent of Formik.

* feat: style text copy modal (TUP-535:2.1.3)

* npx nx format:write

* fix: qr code box was not 200px until image loaded

* fix: remove test logic for msg. about qr alt. code

* fix: pass qr alt. verification code

* feat: allow markup in label e.g. <a>

* fix: markup in label should not be spaced by flex

* fix: let core-styles style form error text

* fix: use FieldWrapper consistently and correctly

Prevents form error messages overlapping qr code messages*.

* These should be called mfa messages. Expect a commit/fix.

* chore: rename `qr-code-message` to `mfa-message`

* chore: remove unused `ButtonWrapper`

* chore: load sibling core-components from rel. path

* chore: remove unused `SectionMessage` import

* fix: resolve "circular dependency"

I.E. Moved FieldWrapper to core-components.

Error: https://github.com/TACC/tup-ui/actions/runs/5650505534/job/15306952950?pr=270

Docs: https://nx.dev/recipes/other/resolve-circular-dependencies

* nx format:write

* fix: loose ends after "circular dependency" fix

* fix: fieldwrapper css duplication too confusing

* chore: remove excess `<span>` tag

* chore: no field wrapper desc unless desc exists

* npx nx format:write

* fix: grammar error from design

* chore: use installed @core-styles, not CDN

* refactor: simplify TextCopyField (no CopyField)

* refactor: simplify TextCopyField (no ButtonWrapper)

* refactor: TextCopyModal hint → <TextCopyModalHint>

* feat: support and add id attr to <Button>

* npx nx format:write

* fix: qr code should resize w/ browser base font

* refactor: simplify qr code styles

* refactor: use variable for qr code size

* fix: increase qr coe size back to 200px not 160px

* fix(a11y): status msg box needs role before msg

https://www.w3.org/WAI/WCAG22/Techniques/aria/ARIA22

> Check that the container destined to hold the status message has a role attribute with a value of status before the status message occurs.

* fix(a11y): no use <label> text for <button> text

I.e. let screen reader read the button text.

* Revert "fix(a11y): status msg box needs role before msg"

This reverts commit f2f325d.

* fix: add and pass id to TextCopyField

* feat(a11y): title text for qr code img

* npx nx format:write

* feat: allow custom `tagName` for `<Message>`

* chore: describe FieldWrapperFormik global css

* refactor: replace FieldWrapper w/ upcoming s-form

Core-Styles will add s-form.

Allows Portals to create well-styled forms w/out copious class names.

* chore: remove FieldWrapper (not Formik)

* refactor:return feild wrap CSS to comp. as global

Return FieldWrapperFormik CSS back to component, but as global.

* npx nx format:write

* refactor: simpler id assignment

* fix: static mfa panel width so mfa-msg is centered

* refactor: no modal for text copty

* fix: restore accientally deleted conditions

* chore: remove component changes moved to PR #277

#277

* chore: remove component changes moved to PR #276

#276

* chore: remove stray changes

* fix: restore TypeScript TextCopyModal

* fix: data.otpkey as var not text

* fix: disable read-only fields, just to be safe

* refactor: use core-styles v2.15, not form.cms.css (#279)

* fix: alt. qr otp label as block instead of inline

Part of the message for QR code alternative OTP was moved to new line.

* fix: add space between mfa msg and section bottom

If section short enough to scroll, MFA message touches section bottom.

* fix: QR loading div was higher than button and img

* fix: darker danger color was not taking effect

The need for !important is likely caused by:
vitejs/vite#4448

Possible solution:
vitejs/vite#4448 (comment)

* fix: core-styles v2.16.2

* fix: remove (now?) unnecessary <br />

The label is display block, so <br /> is not needed to make new line.

* fix: match action spacing, drop related deviations

1. Match spacing between action or action box.
2. Remove deviant font size on an aciton.

* refactor: use SectionHeader / less duplicate code

* chore: nx format:write
Copy link
Member

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Let's do this!

@jarosenb jarosenb merged commit 037b2c4 into main Aug 2, 2023
1 check passed
@jarosenb jarosenb deleted the task/TUP-517--mfa-ui-tweaks branch August 2, 2023 19:20
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