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) Update patient registration page to the new design #754

Closed
wants to merge 6 commits into from

Conversation

Jexsie
Copy link
Contributor

@Jexsie Jexsie commented Jul 8, 2023

Requirements

Summary

  • Update the patient registration page to the new design
  • Update the scrollIntoView block to start
  • Add the active link state
  • Updates the avatar width and height when editing

Screencasts

Before

Screencast.from.2023-07-24.01-15-01.webm

After

Screencast.from.2023-07-24.01-11-37.webm

None.

Related Issue

None.

Other

Relate to Update capture photo widget to the new design

@gracepotma
Copy link
Contributor

Thanks Jessie, when you're able, please add the screenshots showing the software UI before vs after :)

size={size === 'small' ? '48' : '80'}
textSizeRatio={size === 'small' ? 1 : 2}
size={editing ? '120' : size === 'small' ? '48' : '80'}
textSizeRatio={editing ? 3.75 : size === 'small' ? 1 : 2}
Copy link
Contributor Author

@Jexsie Jexsie Jul 13, 2023

Choose a reason for hiding this comment

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

size / textSizeRatio DOC

Copy link
Contributor Author

@Jexsie Jexsie Aug 15, 2023

Choose a reason for hiding this comment

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

what do you say about this @vasharma05?

@Jexsie Jexsie force-pushed the photo branch 2 times, most recently from 5c93e58 to 6f3f7d6 Compare July 23, 2023 21:08
id={attributes.name}
setSelectedValue={setSelectedValue}
selected={selected}
maxLength={attributes.maxCount}

This comment was marked as outdated.

left: 10.5rem;
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

overriding styles for the date input

Copy link
Member

Choose a reason for hiding this comment

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

Why are we having the overrides?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make the date input quite small as the max-characters it can support. Also to align it with the small size of the inputs. If you compare with the identifier inputs below
image

@@ -125,3 +137,7 @@
line-height: 1.34;
margin: 0.25rem 0 0;
}

.numberInput {
width: 13.125rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

giving the number inputs a small size

Copy link
Member

Choose a reason for hiding this comment

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

For now, let it be either 50% of the form width or full width

Copy link
Contributor Author

@Jexsie Jexsie Aug 15, 2023

Choose a reason for hiding this comment

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

This update is majorly to reduce the width of the inputs not to be taking up 100% of the width like the design says. The full input width (300px) looks as below on desktop.

image
image

On dev3
image

@@ -116,3 +118,7 @@
margin: 0;
padding-left: 1rem;
}

.Input {
max-width: 300px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

giving all the inputs a large size

Copy link
Member

Choose a reason for hiding this comment

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

I remember we already have the max-width placed somewhere, can you please find that first?
Thanks!

Copy link
Contributor Author

@Jexsie Jexsie Aug 15, 2023

Choose a reason for hiding this comment

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

I see we were setting the max-width for the whole form here in the infoGrid class, neglecting the sticky column. Here am trying to set the max-width for the inputs
image

Comment on lines 107 to 121
<TextInput
id="yearsEstimated"
type="number"
className={styles.numberInput}
name={yearsEstimated.name}
light
onChange={onEstimatedYearsChange}
labelText={t('estimatedAgeInYearsLabelText', 'Estimated age in years')}
invalid={!!(yearsEstimateMeta.touched && yearsEstimateMeta.error)}
invalidText={yearsEstimateMeta.error && t(yearsEstimateMeta.error)}
value={yearsEstimated.value}
min={0}
required
{...yearsEstimated}
onBlur={updateBirthdate}
/>
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the Layer HOC?
Actually the light prop is deprecated in the new versions and Layer helps in the same.

You can also pass the level prop as well to tweak the theme of the child element. I usually play between 0 and 1 only.

https://react.carbondesignsystem.com/?path=/story/components-layer--custom-level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw they had a parent layer that is actually responsible for the lighting. Just missed out on that light prop.

Comment on lines +21 to +22
display: flex;
flex-direction: column;
Copy link
Member

Choose a reason for hiding this comment

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

If it's a column and no justify-content or align-items, I believe we can totally remove it.

column-gap: spacing.$spacing-05;
display: flex;
flex-direction: column;
gap: spacing.$spacing-05;
Copy link
Member

Choose a reason for hiding this comment

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

gap doesn't work in flex, try using the margins for the child elements

left: 10.5rem;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Why are we having the overrides?

@@ -99,6 +99,8 @@
:global(.omrs-breakpoint-lt-desktop) {
.IDInput {
max-width: unset;
display: flex;
gap: 0.25rem;
Copy link
Member

Choose a reason for hiding this comment

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

Is this working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

surely yes

@@ -116,3 +118,7 @@
margin: 0;
padding-left: 1rem;
}

.Input {
max-width: 300px;
Copy link
Member

Choose a reason for hiding this comment

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

I remember we already have the max-width placed somewhere, can you please find that first?
Thanks!


.linkContainer {
div:last-child {
padding-bottom: 2.3rem;
Copy link
Member

Choose a reason for hiding this comment

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

Weird

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird

this is responsible for the lower grey line after the links and the Cancel Register buttons. Different spacing on tablet and desktop
image
image

@Jexsie
Copy link
Contributor Author

Jexsie commented Sep 21, 2023

@vasharma05 , i think i had this settled?

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.

4 participants