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

fix: tup-571 mfa ui not responsive #315

Closed
wants to merge 2 commits into from

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Sep 6, 2023

Overview

Repair MFA responsive layout CSS so that narrow-screen layout is used.

Related

Changes

  • changed MFA styles to directly use (not compose) responsive styles
  • changed c-step-panels to not assume responsive layout

Testing

  1. Open MFA pairing (or unpairing) view.

  2. Make message appear (e.g. click button to generate QR code).

  3. On wide screen (992px or wider), layout should be "good".
    (click for details)
    1. Verify panels are horizontal.
    2. Verify message is outside first panel.
    3. Verify message is centered (line between panels is at center of message).
    4. Verify message can not exceed width of container.
      To do so, live-edit message text to be long enough to wrap.
    5. Verify all content can be seen when scrolling vertically.
      To do so, make window so short that scrollbar appears.
  4. On narrow screen (991px or narrower), layout should be "good".
    (click for details)
    1. Verify panels are vertical.
    2. Verify message is within first panel.
    3. Verify message is left-aligned.
    4. Verify message can not exceed width of container.
      To do so, live-edit message text to be long enough to wrap.
    5. Verify all content can be seen when scrolling vertically.
      To do so, make window so short that scrollbar appears.

If you notice the horizontal scrollbar, then know that it is fixed in #315.

UI

Wide

normal long message* vertical scroll
wide - normal wide - long message wide - vert  scroll

Narrow

normal* long message* vertical scroll*
normal - normal narrow - long message narrow - vert  scroll
* About the <TextFieldCopy> stretch…

The <TextFieldCopy> will try to stretch to fill available space.

To avoid (subjectively) worst case scenario, wide-screen sets width: max-content on .mfa-message to prevent that, assuming normal-length message text, but with caveats:

  1. A long message text stretches the message, thus giving <TextFieldCopy> opportunity to stretch on wide screen.
  2. <TextFieldCopy> is allowed to stretch on a narrow screen, because were a narrow screen to use width: max-content on .mfa-message, long message text would stretch to fill entire width of screen (which just looked weird to me).

If this solution looks weird to anyone else, tell me and we can let designers decide what they want, which might change the challenge for the better.

Another solution may be to change <TextFieldCopy> to not stretch. (That change can be performed independent of this PR.)

CSS Modules compose rules in @media does not respect query.

So, pull the @media query into Mfa.module.css, thus no need to compose.

This means c-step-panels leaves message and responsive layout to client.

css-modules/css-modules#75
@wesleyboar wesleyboar changed the title fix: un-pairing ui not responsive fix: mfa ui not responsive Sep 6, 2023
@wesleyboar wesleyboar marked this pull request as ready for review September 6, 2023 17:27
@wesleyboar wesleyboar changed the title fix: mfa ui not responsive fix: tup-571 mfa ui not responsive Sep 6, 2023
@wesleyboar
Copy link
Member Author

Moot, because #312 is merged.

@wesleyboar wesleyboar closed this Sep 13, 2023
@wesleyboar wesleyboar deleted the fix/tup-571-mfa-responsive-layout branch October 10, 2023 21:52
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.

1 participant