[terra-clinical-item-view] Set min height so blank displays show up as an empty line

[RayGunY]

Summary

What was changed:
Added a minimum height for the display content. Setting it to 1em for the min because that didn't mess with any of the other snapshots sizing/spacing. When I tried setting it to 1.4285em like the other line heights show up in the browser that changed the screenshots so it looks like we need a smaller line height.

Why it was changed:
Noticed that blank lines were not showing up when passing in an empty item display. The display is technically there but has no height because it's essentially an empty span. See below for reference.

Seems that some users potentially add blank displays to an item view for styling purposes, so we should make sure we're still honoring those blank lines.

Testing

This change was tested using:

• WDIO
• Jest
• Visual testing (please attach a screenshot or recording)
• Other (please describe below)
• No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

• UX review
• Accessibility review
• Functional review

Additional Details

This PR resolves:

No issue has been created for this since it's such a small change, if that needs to change let me know and I can log something.

---

Validation that nothing is read out for those empty displays and that screenreader focus does not shift to the empty newlines.

VO-no-readout-for-empty-displays.mov

[RayGunY]

Based on a support conversation I noticed that when empty displays are passed in to the new true two column layout that a blank ‘newline’ is not showing up like it did before. Looking into this it’s because previously when one column had an empty display, the height from the other display in the same row would be carried across to the empty displays ‘column’. So it looked like a blank newline was returned when an empty display was passed in. In reality the empty display was there but had no height to it because it was essentially just an empty span.

This is what the behavior previously looked like:

So now if someone is using an empty display to create a blank newline and they want to use the trueColumn layout, the blank newline is no longer there (refer to the screenshot I posted in the description). Since we’re doing this by true column now, the height from the populated display in one column does not apply to the empty display across from it.

Setting a min height on the content allows us to keep that ‘newline’ behavior. 
The only issue I’m seeing is that now for the true column layout when there are two empty displays on the same line, a blank newline is being shown across both columns (which was not the previous behavior).

True column layout with a min height added:

You can compare the by row and true column 'same line' examples I added to see the difference.
Before with the by row logic, that newline wouldn’t have shown up in that specific case. That is because the formatting spans the entire row and therefore if the two blank displays are in the same row, they both have no height and therefore no newline is shown (you can refer to the screenshot above for the previous behavior, look at the 'same line' example)

That being said:
For the new true column layout I don’t think there is a good way to conditionally apply the min height based on if two displays are empty and in the same ‘row’.

In practical usage I can’t see why someone would presently have two blank displays passed in if they aren’t going to show up as anything - so I don’t think from a passivity perspective that this change would effect existing consumers. If anything, this would be adding functionality that would allow a user to add a blank newline that spans across the two columns.

I don’t think it’s ideal either way. On one hand if we don’t make this change then anyone who wants to swap to using the trueColumn layout would have UI changes if they are currently using an empty display in one column to create a blank line. On the other hand we would be adding the ability to use displays to create blank lines that appear to span across the two columns- not the end of the world but I don’t think we want to encourage this kind of usage.

[mjpalazzo]

@RayGunY - thanks for the detailed explanation. I support your implementation and think the concern you raised can be addressed in the implementation guide.

[RayGunY]

Ideally I think we would want to apply this min height only when the display is empty. However I wasn't able to figure out how to do that. I tried some variations of the below, but I must be missing something. If needed I think it should be fine to not set min height on just the empty displays, this 1.25em value doesn't mess with any of the other snapshots.

.content:blank {
    min-height: 1.25em;
}

.content:empty {
   &::before {
      min-height: 1.25em;
   }
}

[chenyixuan625]

I am curious if we are able to set it on the implementation side. Maybe adding the display empty check here https://github.com/cerner/terra-clinical/blob/blank-display-height-fix/packages/terra-clinical-item-view/src/ItemView.jsx#L210-L212?

[stale]

This issue has been automatically marked as inactive because it has not had recent activity. It will be closed in seven days if no further activity occurs. Thank you for your contributions.