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

feat: Radio frontend #164

Merged
merged 15 commits into from
Oct 31, 2024
Merged

feat: Radio frontend #164

merged 15 commits into from
Oct 31, 2024

Conversation

mathcolo
Copy link
Collaborator

Asana Task: 📐 Orbit radio UI implementation

Still need to do some testing on mobile (as of this writing I don't have my phone accessible), but otherwise this is radio added to the frontend list display + form pages in a #163-not-ready-yet compatible way. That PR should be merged first.
1
image

Checklist

  • Tests:
    • (x) Has tests
    • ( ) Doesn't need tests
    • ( ) Tests deferred (with justification)
  • Product/Design sign off:
    • (x) Okayed the plan for the feature (e.g. the design files, or the Asana task)
    • ( ) Reviewed the feature as implemented (e.g. on dev-green, or saw screenshots)
    • ( ) No review needed

@mathcolo mathcolo requested a review from a team as a code owner October 23, 2024 21:26
@mathcolo mathcolo added the deploy-to-sandbox deploy this PR to sandbox label Oct 24, 2024
js/models/signin.ts Outdated Show resolved Hide resolved
js/components/operatorSignIn/list.tsx Outdated Show resolved Hide resolved
js/components/operatorSignIn/list.tsx Outdated Show resolved Hide resolved
js/components/operatorSignIn/attestation.tsx Outdated Show resolved Hide resolved
js/components/operatorSignIn/attestation.tsx Show resolved Hide resolved
js/test/components/operatorSignIn/attestation.test.tsx Outdated Show resolved Hide resolved
js/test/components/operatorSignIn/attestation.test.tsx Outdated Show resolved Hide resolved
Copy link
Member

@skyqrose skyqrose left a comment

Choose a reason for hiding this comment

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

A few small comments, none of them are required and none of them require a re-review unless you want one. 👍

js/components/operatorSignIn/attestation.tsx Outdated Show resolved Hide resolved
(!valid || loading) && "opacity-50",
])}
onClick={() => {
onComplete(enteredRadio);
Copy link
Member

Choose a reason for hiding this comment

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

optional refactoring idea: I remember there was a thing about using form onSubmit instead of the button onClick for something else recently. Would it be better to make this button a submit button and do this call in the form's onSubmit handler instead too?

js/components/operatorSignIn/attestation.tsx Show resolved Hide resolved
js/test/components/operatorSignIn/attestation.test.tsx Outdated Show resolved Hide resolved
js/test/components/operatorSignIn/attestation.test.tsx Outdated Show resolved Hide resolved
@mathcolo mathcolo merged commit 60bd233 into main Oct 31, 2024
5 checks passed
@mathcolo mathcolo deleted the pim-radio-frontend branch October 31, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-to-sandbox deploy this PR to sandbox
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants