(feat) O3-3773 Support Summary Tiles component in esm-styleguide commons-lib #1113
Conversation
There was a problem hiding this comment.
- Please look through the checklist on the PR. There are items there that absolutely need to be done.
- Were you able to generate that screen shot using this version of the component?
- I don't really understand the need for this component per se. Where is this intended to be used? How many times do we need it?
| @use '@carbon/styles/scss/colors'; | ||
| @use '@carbon/styles/scss/spacing'; | ||
| @use '@carbon/styles/scss/type'; | ||
| @import '../root.scss'; |
There was a problem hiding this comment.
Don't do this. Follow the guidelines Dennis suggested yesterday on the call (or look through the developer docs)
| <h4 className={styles.title}> {headerTitle} </h4> | ||
| </div> | ||
| {maxRowItems ? ( | ||
| groupedColumns.map((group) => ( |
There was a problem hiding this comment.
Any time you're using a map you should supply a key to the component, either derived from the data or at least using the index of the item.
| padding: spacing.$spacing-04 0 spacing.$spacing-04 spacing.$spacing-05; | ||
| } | ||
|
|
||
| .cardTitle > h4:after { |
There was a problem hiding this comment.
The h4 element has a class, so it seems like this could be better written using .title:after
| .cardTitle > h4:after { | ||
| content: ''; | ||
| display: block; | ||
| width: 2rem; |
There was a problem hiding this comment.
Please use the Carbon tokens for defining spacing wherever possible. This helps ensure that the UI remains consistent with Carbon guidelines.
|
|
||
| .tile { | ||
| height: 100%; | ||
| padding: 1px 0 16px 16px; |
There was a problem hiding this comment.
Nothing over 1px should be specified in px.
| flex-direction: 'row'; | ||
| } | ||
|
|
||
| .widgetContainer { |
There was a problem hiding this comment.
A number of these rules seem to not be used by this component, so they probably don't belong here.
|
@ibacher to answer your question:
It's a useful feature to have a clinical area-specific summary within different clinical views. This was actually a make-or-break requirement for Ampath at the very beginning of O3 :) The user story is, "As a healthcare staff member focused on X condition (eg HIV clinic, MCH clinic, etc), I need to immediately see some very specific information summarized for me, so I can rapidly get the key information relevant to my specialty." Examples here from OHRI: 
|
|
It seems then like this belongs more readily in the patient chart then, rather than the styleguide, more exactly, in the patient-common-lib. |
My thoughts too. If there's no objection to this, I'm going to proceed and and transfer this code to patient-chart. |
No objections from my side, as long as this still works fine with the work @hadijahkyampeire is doing with the config schema. |
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
This PR introduces the summary tile/card summary component to esm-styleguide.
Screenshots
Related Issue
Other