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.

Sign up for GitHub

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

Jump to bottom

organization details screen base #517

Merged
merged 14 commits into from
Nov 29, 2023
Merged

organization details screen base #517

merged 14 commits into from
Nov 29, 2023

Conversation

daileytj
Copy link
Contributor

@daileytj daileytj commented Nov 22, 2023 •
edited
Loading

Fixes BLUI-4849

Changes proposed in this Pull Request:

  • Add organization details screen base

Screenshots / Screen Recording (if applicable)

Screenshot 2023-11-22 at 9 05 51 AM

To Test:

  • yarn start:example
  • navigate to localhost:3000/organization-details-base-screen-demo

@daileytj
Add OrganizationDetailsScreenBase WIP
05197dc
@daileytj
Update example to use dynamic state values
23c7445
@daileytj
Merge branch 'release/derms-auth-workflow' into feature/organization-…
da4f173 
…details-screen

* release/derms-auth-workflow:
  Fix prettier
  Add errorManager wrapper
  revert prop remove
  add label attr
  Remove cursor css style
  Run prettier
  Refactor CreateNewOrgScreenBase and demo
  Add test code in AppRouter
  Remove Next and Previous buttons
  fix grammar
  Add base component for siteOptions
  Add watch script to build on file changes in src
  Clean up linting errors
  Update @types/node
  Update dependencies
@daileytj
Clean up linting errors
1240941
@github-actions github-actions bot added the brightlayer-ui Used to identify Brightlayer UI platform issues for easy filtering label Nov 22, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 22, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit 6e4fcac):

https://blui-react-login--pr517-feature-organization-msnityy3.web.app

(expires Thu, 30 Nov 2023 19:04:01 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 1e9064d2e35ed37fe01e053587ea5f209719a043

@ektaghag-eaton
Copy link
Contributor

  1. No states are under American Samoa and Aland Islands
Screenshot 2023-11-23 at 10 44 18 AM
  1. When I select Aland Islands, it shows an error in the State dropdown. After this, if I select Afghanistan still shows an error in the state dropdown (even if it has a list of states). I think we Need to reset the error state of the State dropdown when the country changes.

@daileytj
Copy link
Contributor Author

daileytj commented Nov 27, 2023 •
edited
Loading

  1. No states are under American Samoa and Aland Islands
Screenshot 2023-11-23 at 10 44 18 AM 2. When I select `Aland Islands`, it shows an error in the `State` dropdown. After this, if I select `Afghanistan` still shows an error in the state dropdown (even if it has a list of states). I think we Need to reset the error state of the `State` dropdown when the country changes.

The spirit of this story is that the options for state and country work. This issue will likely be resolved in the full screen example where the logic for this should actually live. I was trying to create a robust example so that whoever works on the full screen has a good starting point.

edit: I reread this and get what you're saying. Will fix

@daileytj
Refactor country and state autocomplete options
0a3ff44
@daileytj
Merge branch 'release/derms-auth-workflow' into feature/organization-…
058bb3a 
…details-screen

* release/derms-auth-workflow:
  update typography to use variant
  Update login-workflow/src/components/Error/ErrorMessageBox.tsx
  add change log entry
  set return to default
  Blui 4933 error translations
  Removed extra blank line from changelog
  Updated changelog
  Review changes - Reverted the props type of Loginscreen
  Updated package version and changelog
  Added loader to login action
@daileytj
Update state options in example
923896d
@daileytj
Clean up linting errors
9075aca
@daileytj
Update tests to pass
40fcd35
@daileytj
Reset state field error state when country changes
09b9ef4
@JeffGreiner-eaton
Copy link
Collaborator

I get what we are trying here but when we change country value the state is no longer valid and we throw warning...

image

Would the freeSolo prop for autocomplete be of any help here?
https://mui.com/material-ui/react-autocomplete/#free-solo

@daileytj
Copy link
Contributor Author

I get what we are trying here but when we change country value the state is no longer valid and we throw warning...

image

Would the freeSolo prop for autocomplete be of any help here? https://mui.com/material-ui/react-autocomplete/#free-solo

This does take away the warning, but causes some other issues. One, they can enter any value they want so that may not be ideal for validation. There also becomes a type issue as the standard value for state is and object and the freesolo event only outputs a string value. We may be able to work around that as well. I'll look into what we can do here a bit more.

@JeffGreiner-eaton
Copy link
Collaborator

JeffGreiner-eaton commented Nov 27, 2023 •
edited
Loading

I get what we are trying here but when we change country value the state is no longer valid and we throw warning...
image
Would the freeSolo prop for autocomplete be of any help here? https://mui.com/material-ui/react-autocomplete/#free-solo

This does take away the warning, but causes some other issues. One, they can enter any value they want so that may not be ideal for validation. There also becomes a type issue as the standard value for state is and object and the freesolo event only outputs a string value. We may be able to work around that as well. I'll look into what we can do here a bit more.

Would the BLC API validate the required fields and if country is invalid it would kick back like a 400 for bad request or something custom that the error manager would handle?

JeffGreiner-eaton
JeffGreiner-eaton approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@JeffGreiner-eaton JeffGreiner-eaton left a comment

Choose a reason for hiding this comment

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

looks good to me

@daileytj
Copy link
Contributor Author

@ektaghag-eaton this is ready for your review

ektaghag-eaton
ektaghag-eaton approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@ektaghag-eaton ektaghag-eaton left a comment

Choose a reason for hiding this comment

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

Working fine 👍

@ektaghag-eaton ektaghag-eaton merged commit b9f86b0 into release/derms-auth-workflow Nov 29, 2023
7 checks passed
@ektaghag-eaton ektaghag-eaton deleted the feature/organization-details-screen branch November 29, 2023 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ektaghag-eaton ektaghag-eaton ektaghag-eaton approved these changes

@JeffGreiner-eaton JeffGreiner-eaton JeffGreiner-eaton approved these changes

@surajeaton surajeaton Awaiting requested review from surajeaton surajeaton is a code owner

Assignees
No one assigned
Labels
brightlayer-ui Used to identify Brightlayer UI platform issues for easy filtering
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@daileytj @ektaghag-eaton @JeffGreiner-eaton