cucumber / react-components

React components for Cucumber
MIT License
32 stars 10 forks source link

Add support for externalised attachments #353

Closed davidjgoss closed 2 months ago

davidjgoss commented 2 months ago

πŸ€” What's changed?

This PR adds support for externalised attachment content.

Where an attachment message has a populated url field, the <Attachment/> component will now handle that accordingly depending on the media type. The specific variants are:

Also included are a general refactor of <Attachment/> internals to use a more React-y component structure, the addition of an error boundary to handle any errors that happen for a given attachment, and a simpler visual treatment for text/x.cucumber.log+plain attachments:

image

⚑️ What's your motivation?

https://github.com/cucumber/html-formatter/issues/281#issuecomment-1899935418

🏷️ What kind of change is this?

The next step after this will be to modify the HTML formatter implementation(s) to support externalising some kinds of attachment. I'll probably start with JS.

πŸ“‹ Checklist:


This text was originally generated from a template, then edited by hand. You can modify the template here.

netlify[bot] commented 2 months ago

Deploy Preview for cucumber-react-components ready!

Name Link
Latest commit 07bdc16b894bf78624cda2fb99853b246656cebb
Latest deploy log https://app.netlify.com/sites/cucumber-react-components/deploys/667536e140870b0008996818
Deploy Preview https://deploy-preview-353--cucumber-react-components.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mpkorstanje commented 2 months ago

What happens when resources can't be fetched?

davidjgoss commented 2 months ago

What happens when resources can't be fetched?

You get an error shown in place of only that attachment - doesn't affect the rest of the step. See this story https://deploy-preview-353--cucumber-react-components.netlify.app/?story=gherkin--attachment--externalised-error

mpkorstanje commented 2 months ago

Mmh. That error message isn't very helpful. I think it would be good to add some detail to ensure people don't come to us and ask us why their attachments can't be loaded.

For example, Jenkins is commonly used and has rather restrictive CORS settings. Unless configured correctly reports with attachments are always problematic.

So two scenarios that are important to be handled nicely.

davidjgoss commented 2 months ago

Okay, good feedback, let me see if I can make that better.