Skip to content
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

Replace Netlify pull request preview builds with RTD #6030

Merged
merged 10 commits into from
May 20, 2024

Conversation

stevepiercy
Copy link
Collaborator

@stevepiercy stevepiercy commented May 18, 2024

Also temporarily relocate custom.css until @stevepiercy can reconcile legacy styles with current theme.

This PR also uses Plone Sphinx Theme.

Preview:
https://volto--6030.org.readthedocs.build/

I was not able to figure out how to get the Storybook to build on RTD. If anyone has an idea for how to do that, please push to this PR.

This PR needs to be merged before plone/documentation#1667

Temporarily relocate custom.css until @stevepiercy can reconcile legacy styles with current theme
@plone plone deleted a comment from netlify bot May 18, 2024
@plone plone deleted a comment from netlify bot May 18, 2024
@stevepiercy stevepiercy marked this pull request as draft May 18, 2024 11:24
@stevepiercy stevepiercy marked this pull request as ready for review May 18, 2024 11:39
@ichim-david
Copy link
Member

@stevepiercy what are the advantages of having the docs with RTD as opposed to netlify?
I see one dissavantage with the storybook build.
I'm trying to understand the pros and cons of this move and why we need todo it now.

@stevepiercy
Copy link
Collaborator Author

stevepiercy commented May 18, 2024

See plone/plone-sphinx-theme#4 (comment). tl;dr: this PR is only for pull request preview builds. The new theme requires Python 3.9+, and Netlify supports only 2.7 and 3.8.

The deployed Storybook at https://6.docs.plone.org/storybook/ is on our infrastructure, not on RTD or on Netlify. That will continue to render.

I am fairly sure that there is some incantation to build Storybook in pull request previews. I just don't know what it is. RTD has grown up from its roots, allowing full control of the build process.

@stevepiercy
Copy link
Collaborator Author

@stevepiercy
Copy link
Collaborator Author

@ichim-david
Copy link
Member

Nailed it! https://volto--6030.org.readthedocs.build/storybook/

@stevepiercy I don't like the fact that read the docs is so aggressive with their watermark to advertise their service
within the Storybook build.
I can say ok one logo is fine in the control area toward the right but you get another one also within the
story section.
Ex:
https://volto--6030.org.readthedocs.build/storybook/?path=/story/public-components-eventdetails--event-details&globals=backgrounds.grid:!false;viewport:mobile1

This makes it hard to see the story due to content being hidden.
read-the-docs-mobile

@stevepiercy
Copy link
Collaborator Author

stevepiercy commented May 19, 2024

There is a warning that can be dismissed with a click of the X.

The RTD flyout menu is not advertising at all, but has useful links and a search feature.

Screenshot 2024-05-19 at 3 32 50 AM

It can be repositioned or completely hidden with custom CSS. We don't need it on PR previews, unless you want to use any of its links. https://docs.readthedocs.io/en/stable/flyout-menu.html#defining-where-the-flyout-menu-is-injected

@stevepiercy
Copy link
Collaborator Author

In any case, style issues are beyond the scope of this PR. All style issues need to have a separate issue in https://github.com/plone/plone-sphinx-theme/issues/ and discussed.

It may be that one project wants one style, and another project wants a conflicting style, such as whether or not to display the RTD flyout menu. We already learned that maintaining custom CSS across 5 projects was unsustainable.

@ichim-david
Copy link
Member

In any case, style issues are beyond the scope of this PR. All style issues need to have a separate issue in https://github.com/plone/plone-sphinx-theme/issues/ and discussed.

It may be that one project wants one style, and another project wants a conflicting style, such as whether or not to display the RTD flyout menu. We already learned that maintaining custom CSS across 5 projects was unsustainable.

@stevepiercy having that popover the storybook content is not styling, it's a functionality issue.
This got highjacked into the original pull request for the docs simply because you did also the storybook.
I'm +1 for the RTD deployment of docs and -1 for storybook until that popup RTD button is removed from the storybook.
Ex https://plone-components.netlify.app/ there is no pollution of storybook infrastructure.

@stevepiercy
Copy link
Collaborator Author

The flyout can be hidden with a style, so it is a style issue. The preview of pull requests is merely a convenience. There are workarounds for this optional feature, either by building the Storybook locally via make storybook-build or use what is in production. We'll agree to disagree.

Alternatively, the Storybook build could remain on Netlify, isolated from the MyST docs, but I have no desire to support a yet another build environment.

I encourage you to create an issue in https://github.com/plone/plone-sphinx-theme/issues/, if the flyout is something that you think needs to be addressed.

@sneridagh
Copy link
Member

@ichim-david @stevepiercy I'm +1 to find a better place to live to all Plone StoryBooks, centralized, no clue how we should build it, but yeah. I talked briefly to @ericof and @fredvd about it at some point the last months.

@ichim-david are you ok in merging this in the meanwhile?

@sneridagh sneridagh merged commit f03e4b7 into main May 20, 2024
71 checks passed
@sneridagh sneridagh deleted the pull-request-preview branch May 20, 2024 14:31
@stevepiercy
Copy link
Collaborator Author

I'm +1 to find a better place to live to all Plone StoryBooks, centralized, no clue how we should build it, but yeah. I talked briefly to @ericof and @fredvd about it at some point the last months.

@sneridagh I assume you mean Storybooks for production like https://6.docs.plone.org/storybook/, not pull request previews.

List the Storybooks. We need an inventory so we know exactly what we're talking about. Do they all exist in the plone/volto monorepo, or are they scattered about? Once we have that, then:

  1. What's the purpose of hosting a given Storybook?
  2. I don't want to slow down production deployments and pull request previews with many Storybooks. I only want to update a Storybook when its files change. Volto Storybook takes about 3.5 minutes to build.
  3. We can extend existing processes and reuse infrastructure that we use for production deployments of Documentation.

Maybe turn this comment into a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants