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

Mobility Profile #1090

Merged
merged 44 commits into from
Dec 11, 2023
Merged

Mobility Profile #1090

merged 44 commits into from
Dec 11, 2023

Conversation

binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Dec 6, 2023

Description

Works with ibi-group/otp-middleware#200.

This PR includes changes for enabling in config the mobility profile pages in the settings pages,
whether it is on creating new accounts or existing accounts.

To enable mobility profiles, simply add a line in config.yml with:

mobilityProfile: true

Behind the scenes, the code for UserAccountScreen is heavily modified, and the "wizard" concept (previously SequentialPaneDisplay) is used more widely for the mobility profile setup screen.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

Copy link
Member

@AdrianaCeric AdrianaCeric left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! As a next step, it is not clear the settings the user selected from the main settings page. It would be great that after parameters are defined that they are shown on the main page like this:
image
Define Your Mobility Profile would be changed to Edit Your Mobility Profile after the user has edited it >0 times

@AdrianaCeric AdrianaCeric removed their assignment Dec 8, 2023
Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Looks really great! I do agree with Adriana's comment - ideally users should be able to see their selected settings without entering the wizard. Overall though, looks really good and just a couple of small questions

]

const DeviceOption = styled.span`
display: block;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we switched this to flex and add a gap? These labels and inputs are very close together.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 23edbc8.

<Wizard
activePaneId={activePaneId}
formikProps={formikProps}
pages={['mobility1', 'mobility2']}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change these to "assistive devices" and "limitations"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I did in b1ad318.

this._focusHeader()
} else {
// Display a toast to acknowledge saved changes
// (although in reality, changes quietly took effect in previous screens).
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, cool!

@binh-dam-ibigroup
Copy link
Collaborator Author

It would be great that after parameters are defined that they are shown on the main page like this...

Good idea, I added a basic summary in a3a0a85.

@binh-dam-ibigroup
Copy link
Collaborator Author

Define Your Mobility Profile would be changed to Edit Your Mobility Profile after the user has edited it >0 times

Updated in d680855

@binh-dam-ibigroup binh-dam-ibigroup merged commit 9a5b19a into dev Dec 11, 2023
9 checks passed
@binh-dam-ibigroup binh-dam-ibigroup deleted the mobility-profile branch December 11, 2023 18:01
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.

3 participants