Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

[clinical-result] Add doc example for header/cell #877

Merged
merged 5 commits into from
Jul 14, 2023
Merged

Conversation

RayGunY
Copy link
Contributor

@RayGunY RayGunY commented Jun 27, 2023

Summary

What was changed:
We need to add examples for clinical result of the usage for Flowsheet Cell, Time Header Cell, and Name Header Cell within a semantic html table.

I added a semantic table example instead of changing all of the existing examples in case consuming teams continue to use this component in a non-semantic way. We of course want our users to use this with appropriate semantic html, so I will be adding guidance on that in our accessibility page that is being created in another PR.

Why it was changed:
The look/behavior of these cells are different when they are used within semantic html tables, so we need to give examples of what that will look like.

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:

UXPLATFORM-9205


Thank you for contributing to Terra.
@cerner/terra

@RayGunY
Copy link
Contributor Author

RayGunY commented Jun 28, 2023

Terra site for these changes: https://engineering.cerner.com/terra-clinical/pull/877

@RayGunY RayGunY marked this pull request as ready for review July 11, 2023 13:05
@RayGunY
Copy link
Contributor Author

RayGunY commented Jul 11, 2023

Have a WDIO issue I'm looking into for flowsheet result cell semantic table, discussing that on this PR here: #881 (comment)

Other than that this can be looked at though.

@vinaybhargavar
Copy link
Contributor

@RayGunY There is a WDIO test failing. Is it related to your changes?

@RayGunY
Copy link
Contributor Author

RayGunY commented Jul 12, 2023

@RayGunY There is a WDIO test failing. Is it related to your changes?

@vinaybhargavar Kind of, I'm working on it right now - it's being fixed in the defect PR. It's a lot to explain, so a breakdown can be found in this comment: #881 (comment)

Basically the code on my side is correct and if you go look at the flowsheet semantic table test on the PR site you'll see it looks the way we want it to. The snapshot is failing here because I haven't pushed out the re-generated snapshot. You can ignore that failure for now and take a look at the rest of the code.

Edit: removing that test from the flowsheet-result-cell-spec.js file so it doesn't generate wdio. We're taking care of that in this defect PR: #881

Since it's causing this PR to be put on hold we're removing it so it doesn't block this.

@aalexanderj
Copy link
Contributor

Looks good, +1 after resolving the WDIO issue.

@vinaybhargavar
Copy link
Contributor

@RayGunY There is a WDIO test failing. Is it related to your changes?

@vinaybhargavar Kind of, I'm working on it right now - it's being fixed in the defect PR. It's a lot to explain, so a breakdown can be found in this comment: #881 (comment)

Basically the code on my side is correct and if you go look at the flowsheet semantic table test on the PR site you'll see it looks the way we want it to. The snapshot is failing here because I haven't pushed out the re-generated snapshot. You can ignore that failure for now and take a look at the rest of the code.

Sounds good. +1 after resolving the WDIO issue.

@github-actions github-actions bot temporarily deployed to preview-pr-877 July 13, 2023 14:56 Destroyed
@@ -1,4 +1,4 @@
import React, { useRef, useState, useEffect } from 'react';
import React, { useRef, useState, useLayoutEffect } from 'react';
Copy link
Contributor Author

@RayGunY RayGunY Jul 13, 2023

Choose a reason for hiding this comment

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

Fixes in this file are related to the flicker defect being dealt with in this PR: #881

Adding them to this PR because otherwise we'll have the same issue. Same things with the flowsheet css file.

@RayGunY
Copy link
Contributor Author

RayGunY commented Jul 13, 2023

@scottwilmarth @sdadn @sycombs This PR is good to be looked at. It's touching some of the same stuff as the defect PR so there is some overlap you might see. Trying to unblock this PR so we can wrap up clincial result phase 1, so the failing wdio test has been removed as that will technically be added in the defect PR.

Defect PR for reference: #881

@sdadn
Copy link
Contributor

sdadn commented Jul 13, 2023

@RayGunY Does this PR have the same wdio issue as #881 ?

@RayGunY
Copy link
Contributor Author

RayGunY commented Jul 14, 2023

@RayGunY Does this PR have the same wdio issue as #881 ?

@sdadn we removed it from this PR since we'll handle it in the other, so this one should be good to go

@sdadn sdadn merged commit 3325f7c into main Jul 14, 2023
21 checks passed
@sdadn sdadn deleted the UXPLATFORM-9205 branch July 14, 2023 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants