feat: Content updates to Candidate Review page #1186
Conversation
There was a problem hiding this comment.
I am not entirely satisfied by this placement but there wasn't a strong enough organization strategy in place for content-related utilities used by more than one component. Ideally, it would be a useMemo custom hook but that would've required some restructuring. Open to alternatives but this seemed acceptable for now.
There was a problem hiding this comment.
I suppose something like this could go into client/common as its own component but this is okay. Tweaks to organization of these shared components could be its own task.
it would be a
useMemocustom hook
Good thought for the future!
There was a problem hiding this comment.
A detail for consideration was missed when describing what should be done here. Since the Candidate Test Plan Run page will list the results of several Browsers if found, how the Run History render function is being called here will hide results for all the n+1 reports, as shown in the screenshot.
This may require some restructuring of the getTestersRunHistory function to get them into the single disclosure. But having separate Run History disclosures so as to not make extensive changes there also seem okay to me. ie.
- Test Instructions
- Test Results for Firefox
- Run History for Firefox
- Test Results for Chrome
- Run History for Chrome
There was a problem hiding this comment.
I suppose something like this could go into client/common as its own component but this is okay. Tweaks to organization of these shared components could be its own task.
it would be a
useMemocustom hook
Good thought for the future!
Ah good catch! I had to get my DB into a different state to see this case. I decided to go ahead and turn this into a dedicated component. I made the |
| <a href={`/test-review/${testPlanVersion.id}`}> | ||
| {`${ | ||
| testPlanVersion.title || testPlanVersion.testPlan?.directory || '' | ||
| } ${testPlanVersion.versionString}`} | ||
| </a> |
There was a problem hiding this comment.
Note to ourselves to also bring this to the TestRun page as well!
|
@stalgiag there is now a merge conflict here but feel free to merge once it's resolved |
This PR does the following:
test-review/page to the Candidate Review pageaddresses #1166
addresses #1167
addresses #1168