Skip to content

docs: Overhaul intro docs to use the ESM build #1911

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

Merged
merged 1 commit into from
May 14, 2021

Conversation

floryst
Copy link
Collaborator

@floryst floryst commented May 14, 2021

Context

This PR updates the documentation to use the new @kitware/vtk.js ESM build. The old webpack-dependent approach is still supported, but its use in new projects is discouraged.

Fixes issue #1867

Changes

This only introduces documentation changes.

@floryst floryst requested a review from jourdain May 14, 2021 16:38

import '@kitware/vtk.js/Rendering/Profiles/Geometry';

import vtkFullScreenRenderWindow from '@kitware/vtk.js/Rendering/Misc/FullScreenRenderWindow';
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good, but it might be interesting to share/include a React component that map to a vtkRenderWindow...
Maybe we can bundle those in the code without importing react or what not? (Same for vue?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or we could just point people to check out react-vtk-js. But yeah, having extra docs for that could be a useful thing down the road. For this tutorial, I'm less concerned about it, but it is an option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree, for the current step it is good.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is just that I don't want users use the fullscreenRW in production when their goal is to have the 3D not in fullscreen.

Copy link
Collaborator

@jourdain jourdain left a comment

Choose a reason for hiding this comment

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

LGTM we should merge it sooner rather than later

@floryst floryst merged commit 02b8e16 into Kitware:master May 14, 2021
@floryst floryst deleted the esm-docs branch May 14, 2021 20:16
@github-actions
Copy link

🎉 This PR is included in version 18.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Automated label label May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Automated label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants