Skip to content

feat: nfc scanner#415

Merged
katiez667 merged 9 commits intosc0v:masterfrom
tomas-goncalves:feat/nfc-scanner
Apr 4, 2026
Merged

feat: nfc scanner#415
katiez667 merged 9 commits intosc0v:masterfrom
tomas-goncalves:feat/nfc-scanner

Conversation

@tomas-goncalves
Copy link
Copy Markdown
Contributor

Add a button that enables NFC scanning on browsers where NFC is allowed. Tested on the Office Android Phone. Works like a barcode reader after permission is granted.

@merichar
Copy link
Copy Markdown
Member

merichar commented Apr 2, 2026

Double-click race: the click handler doesn't guard against being triggered twice before ndefActive
is set and the button hides. Add if (ndefActive) return; at the top of the click handler to prevent
two NDEFReader instances from spinning up simultaneously.

Byte order assumption: convertSerialNumber reverses bytes assuming LSB-first order (correct for
MIFARE). Add a comment noting this assumption so it's not mysterious if we ever add a different tag
type.

FAB visibility: the button will appear for all users on NFC-capable browsers, not just SCC users.
Backend permissions handle this correctly so it's not a security concern, just a heads-up that
non-SCC users on Android Chrome will see it and get no feedback when their scan does nothing. If we
want to scope it, the lightest touch would be a data-scc attribute on that the JS checks
before appending the FAB.

@merichar
Copy link
Copy Markdown
Member

merichar commented Apr 2, 2026

By the way, since literally only 2 devices are expected to use this feature, but 100-ish lines are now in everyone's bundle, the cleaner approach would be a Stimulus controller attached to a partial that's only rendered for
SCC users: that way the JS is only initialized on the relevant pages and the scoping is handled in
Ruby where the role logic already lives.

@merichar
Copy link
Copy Markdown
Member

merichar commented Apr 2, 2026

SCC logic in the application layout: the main layout now has awareness of a specific user role, which tends to creep. If more role-specific UI gets added this way it becomes hard to reason about.

JS is still global: the partial gates the button but nfc.js is still loaded for everyone via application.js. The JS and the HTML are now out of sync in terms of who they target.

Consider something in the vein of:

<% content_for :head do %>
  <%= javascript_import_module_tag "custom/nfc" %>
<% end %>

@katiez667 katiez667 self-requested a review April 4, 2026 05:20
@katiez667 katiez667 merged commit 36f4791 into sc0v:master Apr 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants