Skip to content

[FLORA-288] Hide 2FA/TOTP input on login page by default#1141

Open
kd-e wants to merge 8 commits into
developmentfrom
hide-2FA-input
Open

[FLORA-288] Hide 2FA/TOTP input on login page by default#1141
kd-e wants to merge 8 commits into
developmentfrom
hide-2FA-input

Conversation

@kd-e

@kd-e kd-e commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

  • Made the 2fa input hidden by default, visible on checkbox change
  • Fixed the Alpine js binding for ariaControls_: aria-controls can be just a regular attribute
  • Fixed/added spacing between the checkbox and label

Contributor checklist

@linear

linear Bot commented Jun 26, 2026

Copy link
Copy Markdown

FLORA-288

@kd-e kd-e added the frontend Frontend concerns label Jun 26, 2026
@elland elland requested a review from TixieSalander June 26, 2026 15:35
@elland elland marked this pull request as ready for review June 26, 2026 15:38

@elland elland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Haskell side looks good to me. Testing locally works as expected. :shipit:

@tchoutri tchoutri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested locally

@TixieSalander TixieSalander left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Except this little thing, looks good to me! 👌

, method_ "POST"
, class_ formClasses
, xData_ "{ open: false }"
, xInit_ "const sync = () => { open = $refs.useTotp.checked }; sync(); window.addEventListener('pageshow', sync);"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Considering Flora is a MPA with no client-side navigation stuff to be careful with, I think it can be simplified by directly calling the update once on Alpine's init:

Suggested change
, xInit_ "const sync = () => { open = $refs.useTotp.checked }; sync(); window.addEventListener('pageshow', sync);"
, xInit_ "open = $refs.useTotp.checked"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This works for init but on back/forth open doesn't get updated even though the checked state persists:

Screenshot 2026-07-02 at 13 08 54

I'm still getting used to alpine's directives but I think maybe this would be better and more correct than the eventListener? 🙈

, xData_ "{ open: false, sync() { this.open = this.$refs.useTotp.checked; }}"
, xInit_ "sync()"
, xOn_ "pageshow.window" "sync()"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thats weird it should still work without any global event listener.

  • x-init set the open variable to the checkbox's state on page load
  • x-model="open" on the checkbox set a 2way data binding between the checkbox's value and the open variable, so the open variable is updated automatically when the checkbox's state is changed
  • and the x-bind:hidden and x-bind:inert are updated when the open variable changes

After applying the suggested change above and nothing else, It works on my end. Did you made other changes since your last commit by any chance? :s

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, no other changes on my end. I did some more testing with your suggestion and it works on Firefox and Safari on macOS but not on Chrome/Vivaldi xD fun! Firefox even persists checked+hidden/inert on page reload, except on first load with the checkbox unchecked, then the 2fa code input wrapper flashes. Hilarious

Screen.Recording.2026-07-03.at.08.51.11.mov

I'll apply your suggestion though to keep things simple 👍

@kd-e kd-e force-pushed the hide-2FA-input branch from 01f3d90 to 20c6a1c Compare July 3, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Frontend concerns

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants