-
Notifications
You must be signed in to change notification settings - Fork 12
Add preliminary support for rendering run hooks #408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cucumber-react-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. Looks good, but some fairly minor feedback and suggestions.
I noticed on the story for BeforeAll errors that the <FilteredDocuments>
component renders "nothing found" when really there are just no test cases at all - I'm reworking the search/filtering behaviour on another branch so I'll aim to take care of this over there, but I'm glad this change shone a light on it.
|
||
export const RunHooksList: FunctionComponent<{ children: ReactNode }> = ({ children }) => { | ||
return ( | ||
<ol aria-label="RunHooks" className={styles.hooks}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave it to consumers to decide if/how to label this section with e.g. headings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How were you thinking? EG. a aria-label prop?
Leave this to the consumer.
This served little to no purpose.
🤔 What's changed?
A list below GherkinDocument, showing
BeforeAll(..)
andAfterAll(..)
hooks.⚡️ What's your motivation?
Errors in
BeforeAll(..)
leads to a merely empty document - this shows why, in addition to adding support for presenting run hooks attachments.🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
Anything, really. I'm new to the project and even ladle.
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.