cerner / terra-core

Terra offers a set of configurable React components designed to help build scalable and modular application UIs. This UI library was created to solve real-world issues in projects we work on day to day.
http://terra-ui.com
Apache License 2.0
183 stars 165 forks source link

Demographics Banner - Wrapping Behavior #1399

Closed ChelseaRyanAnderson closed 6 years ago

ChelseaRyanAnderson commented 6 years ago

Issue Description

screen shot 2018-04-03 at 4 24 33 pm

DOB year is wrapping to second line, even with plenty of room.

Issue Type

Expected Behavior

The DOB shouldn't wrap if there is space to accommodate it. If there's an instance where data on the right is pushing data on the left to wrap, the entire line should wrap down to the next line to avoid this behavior:

screen shot 2018-04-03 at 4 26 38 pm

Current Behavior

With this amount of space on the right side, the year shouldn't wrap.

Steps to Reproduce

https://kaiju.cerner.com/projects/divine-amano-shiratori-GtgdCA/workspaces/twilight-king-kong-LWU15w/preview

Environment

JakeLaCombe commented 6 years ago

For mobile devices, you actually will not see that behavior, as we stack all of the data as opposed to having some one the left, and others on the right. Here is a screenshot of what it looks like. This makes me think Kaiju will need to have some kind of way to address for this in it's right panel. screen shot 2018-04-04 at 10 02 32 am

As for terra work on the component, I can understand the cause of concern of having the data wrap like it is. We could potentially put the dates on a new line which could help with some of that issue. There will always be date wrapping though, as September is a large word for mobile screen devices.

JakeLaCombe commented 6 years ago

Engaging @neilpfeiffer for UX assistance.

StephenEsser commented 6 years ago

@JakeLaCombe

Can you elaborate a little more on This makes me think Kaiju will need to have some kind of way to address for this in it's right panel?

Kaiju applies no opinionated CSS to any components ( with the exception of applying a 100% height to the root container ). Any modification to the demographics banner would need to be exposed through the DemographicsBanner itself.

JakeLaCombe commented 6 years ago

Is that right handed X panel designed to be what the component looks like on mobile devices? If so, is there a way to apply the media query styles to that panel?

StephenEsser commented 6 years ago

This appears to be a DemographicsBanner within the detail of a SlidePanel. It's not a mobile view. The DemographicsBanner may need to be enhanced to be responsive to its container size instead of the window.

ChelseaRyanAnderson commented 6 years ago

@neilpfeiffer Feel free to correct me, but the wrapping displayed below is difficult to read and looks pretty jumbled.

screen shot 2018-04-04 at 11 04 32 am

@JakeLaCombe @StephenEsser Yes, this is a demo banner within a side panel, not mobile. At the smallest view via chrome, it displays like so:

screen shot 2018-04-04 at 11 06 34 am
JakeLaCombe commented 6 years ago

That's what I'm thinking with the demographics banner as well. It should probably respond to the size of it's parent container, not container of the window.

ChelseaRyanAnderson commented 6 years ago
screen shot 2018-04-04 at 11 17 20 am

I noticed that on the site the banner is behaving properly at 320px, the width of the side panel. So that seems like it would be the case @JakeLaCombe

JakeLaCombe commented 6 years ago

Looks like we have most of the implementation there as well. So we have two changes to make here.

  1. Demographics Banner is responsive to window. First class that property to allow for developers to set it based on their needs.
  2. Maybe investigate putting the entire date on a new line for small viewports.
StephenEsser commented 6 years ago

Is there a situation where a user would want the demographics banner to be responsive to the window and not the container?

neilpfeiffer commented 6 years ago

I'm thinking we only need to have it be responsive to container. @JakeLaCombe I can't see of any reason that we maintain the _Large layout when in a small container, but on a larger window.

just need to update this: https://github.com/cerner/terra-core/blob/c7a0c913b0b3ff6c43733e967dcb1aa2f0711414/packages/terra-demographics-banner/src/DemographicsBannerDisplay.jsx#L93

(I think that was something we got close with initial implementation, but never verified afterward.)

\ \ Also for reference with @ChelseaRyanAnderson very tall/skinny screenshot:

... the wrapping displayed below is difficult to read and looks pretty jumbled.

A demo banner would be/should be never be presented at at 150px width and so no matter what layout is used, wrapping is going to happen and the look will always be unideal, but it would be better using the _Small layout with everything left aligned and wrapping.

Special note - the current _Small layout also hides the patient photo/avatar.

Thanks for the assistance @JakeLaCombe and @StephenEsser.

ChelseaRyanAnderson commented 6 years ago

@neilpfeiffer Yeah, I didn't realize how small that view was until I messed around with it on the site some more! Like you said, it shouldn't be presented at that size, so I understand that it's not going to be ideal at such a small size.

neilpfeiffer commented 6 years ago

Note to everyone cc: @ChelseaRyanAnderson @JakeLaCombe @StephenEsser

We may need to run some quick tests on what should be correct breakpoint. It currently switches to the _Small layout at 767px (Small breakpoint) and below, not 543px and below (Tiny breakpoint).

Which means after the switch proposed above, that any demo-banner in a modal or side-panel, on tablet, will use the left aligned (more lines) and no patient photo version of the demo banner. Need to see if that is okay, or if we need to also change it to tiny breakpoint, add another layout (not ideal), and test accordingly.

JakeLaCombe commented 6 years ago

We can have it just respond to the parent then. Once you run that test, I should be able to submit a pull request.