DOM fixture updates #13368
DOM fixture updates #13368
Conversation
This commit adds a homepage to the DOM fixtures that includes browser testing information and asynchronously loads fixtures. This should make it easier to find DOM testing information and keep the payload size in check as we add more components to the fixtures.
| </table> | ||
| </section> | ||
| <section> | ||
| <h2>How do I test browsers I don't have access to?</h2> |
There was a problem hiding this comment.
I like that section a lot! Maybe we can also list some free options if money is an issue. For example:
- Microsoft provides the option to download free virtual machines with various Internet Explorer versions: https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/
- You can download a legacy version of Chromium by looking up the base position in https://omahaproxy.appspot.com/ and using that number in https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html.
- Maybe that's a little to complicated? We can skip the first step and hardcode
369907as the base position? - Alternatively we can link to https://www.chromium.org/getting-involved/download-chromium
- Maybe that's a little to complicated? We can skip the first step and hardcode
- Firefox ESR versions can be downloaded here: https://www.mozilla.org/en-US/firefox/organizations/
I guess that's it. For Safari you'd need a VM to an old macOS version which will require running macOS as well.
There was a problem hiding this comment.
Those are all great ideas! I'll update this section with these recommendations.
| const {fixturePath} = this.props; | ||
|
|
||
| try { | ||
| let module = await import(`.${fixturePath}`); |
There was a problem hiding this comment.
Haha, yeah... Though we still need to support React 14/15 for this page (but maybe only 15, we should talk about that).
aweary
left a comment
aweary
left a comment
There was a problem hiding this comment.
Great improvement! LGTM with a couple non-blocking nits 👍
| <tr> | ||
| <th>Browser</th> | ||
| <th>Versions</th> | ||
| <th>Why/Notes</th> |
There was a problem hiding this comment.
Minor UX nit: this "Why/Notes" column is mostly empty. The few notes we have would likely be better as footnotes
There was a problem hiding this comment.
Great idea. Done.
| </li> | ||
| </ul> | ||
| <p> | ||
| These services cost money, however a few maintainers have access to a |
There was a problem hiding this comment.
We should really make sure all maintainers have access to a subscription. @sophiebits is there a way we can get a shared account for one of these services that external collaborators can also use?
There was a problem hiding this comment.
BrowserStack is free for selected open source projects: https://www.browserstack.com/open-source/
Since Angular qualifies we might as well?
There was a problem hiding this comment.
I’m not sure if we have a team account now but if it would be helpful to y’all we can get one. @gaearon?
There was a problem hiding this comment.
I think we had one but we cancelled the plan for automation. We can probably go on a different plan?
There was a problem hiding this comment.
@philipp-spiess I reached out to BrowserStack about it awhile ago, and they require projects to include an endorsement/link in the README to get the free option. Which, at least at the time, wasn't something we were able to do.
A team account would be great! We just need the Live functionality for manual testing.
| const {Fixture, error, isLoading} = this.state; | ||
|
|
||
| if (isLoading) { | ||
| return <p>Awaiting fixture...</p>; |
There was a problem hiding this comment.
Switching between pages is a little jarring with this. Since we do full page reloads every time you select a new fixture it always flashes "Awaiting fixture..." before rendering. It might be better to just render null
| } | ||
|
|
||
| if (error) { | ||
| return <p>Fixture at path {fixturePath} could not be loaded.</p>; |
There was a problem hiding this comment.
I debated back and forth with including the stack trace too, but it felt noisy when this is more or less a 404 every time
| <p> | ||
| These services cost money, however a few maintainers have access to a | ||
| subscription. There is no obligation to subscribe to these services; | ||
| feel free to ping a maintainer or mention that more testing is |
There was a problem hiding this comment.
I wonder if ping is widely understood. Changing this too.
|
Updated with everyone's feedback |
| const {fixturePath} = this.props; | ||
|
|
||
| try { | ||
| let module = await import(`.${fixturePath}`); |
|
Will merge after CI |
|
Thanks everyone! |
7 participants
After @philipp-spiess's last PR, I found myself wishing that browser testing recommendations were easier to find. This commit adds a home page to the DOM fixtures that includes testing information.
Also, we're getting a lot of fixtures! I've changed the fixture switch component to asynchronously load fixtures as needed. If we ever publish the fixtures officially, this should cut the payload size a fair bit.
Finally, the fixture styles were never build to accommodate long-form text content. This PR also adds a general style update to make text a bit nicer.
Test plan
Take a look at http://react-dom-home.surge.sh