codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
24 stars 24 forks source link

Issue 500/sign in modal #537

Closed xscottxbrownx closed 7 months ago

xscottxbrownx commented 7 months ago

This PR:

Resolves #500

1. Changes styling of "Sign In" button to match "Log In" button 2. shrinks size of input on the SignInModal, so no side scrolling occurs

Screenshots (if applicable):

Screenshot 2023-11-21 at 8 33 46 PM Screenshot 2023-11-21 at 8 33 52 PM
xscottxbrownx commented 7 months ago

@leekahung @andycwilliams


3 questions for you both:

1) Is there any need to have the button and modal say "Sign In" vs the "Login" that the desktop sees?

2) What do you think of making the "Login" button green only when it's in the modal?

3) Do we need the subtitle text "Welcome to PASS. Sign in below"

leekahung commented 7 months ago

1) Is there any need to have the button and modal say "Sign In" vs the "Login" that the desktop sees?

  • If not, I can cut a little code down

Technically, it's not a login button in mobile, so having it named different seems right to me. The login in the modal is the proper one. It was why I originally made the sign up button explicitly different with wording and coloring to point that out.

2) What do you think of making the "Login" button green only when it's in the modal?

  • Just to match all other modals of "cancel" being red, and "continue" being green

Yeah, this is fine. If we're switching up the coloring for the Login Button think we can leave the sign up button outside red.

3) Do we need the subtitle text "Welcome to PASS. Sign in below"

  • Most other modals like this do not have it, and I'm not sure it provides any value at all

We could change it to, "Please select a Solid Server for login" or the like.

xscottxbrownx commented 7 months ago

The second was related to the <Autocomplete>, see clip. Was wondering on how we wish to manage the internal vertical scrolling of the modal if users were to select say the servers lower in the recommended list. Could address this in this PR or in another one, either is fine with me.

As for a solution, the first thing that comes in mind was to have the <Autocomplete> vertical scroll instead, but open to alternatives.

Want to make separate issue?

leekahung commented 7 months ago

The second was related to the <Autocomplete>, see clip. Was wondering on how we wish to manage the internal vertical scrolling of the modal if users were to select say the servers lower in the recommended list. Could address this in this PR or in another one, either is fine with me.

As for a solution, the first thing that comes in mind was to have the <Autocomplete> vertical scroll instead, but open to alternatives.

Want to make separate issue?

Sounds good to me. Think we could have it set up after the merge.

andycwilliams commented 7 months ago

I think this is all good now. I double approve.