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

Update Pluggables Framework Documentation #4948

Merged
merged 10 commits into from
Jul 16, 2023

Conversation

Akshat2Jain
Copy link
Member

@Akshat2Jain Akshat2Jain commented Jul 5, 2023

Description:

This PR updates the documentation for the Pluggables framework to provide a clearer understanding of its capabilities and limitations. The changes aim to improve the clarity and accuracy of the information provided to developers.

Changes Made:

Added a note highlighting that Pluggables are not compatible with server-side rendering (SSR) and are better suited for visual enhancements rather than handling critical data during the loading process.

This Pr fixes #4735

@netlify
Copy link

netlify bot commented Jul 5, 2023

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 73b5016
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64b256cbf45bc1000862db39
😎 Deploy Preview https://deploy-preview-4948--volto.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.

@Akshat2Jain
Copy link
Member Author

The major change is at line no 60 .

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Revert html_meta changes and some grammar fixes. Thank you!

'description': "The Pluggables framework give you insertion points to push components to other components in an 'out of tree' fashion, like React's Portal component, but with vitamins."
'property=og:description': "The Pluggables framework give you insertion points to push components to other components in an 'out of tree' fashion, like React's Portal component, but with vitamins."
'property=og:title': 'Pluggables framework'
'keywords': 'Pluggables, framework, React, portal'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert the above changes, and configure your editor accordingly.

@@ -57,13 +57,15 @@ Internally, the `<PluggablesProvider>` keeps a record of `Pluggables` and `Plug`
this is achieved by having the `<Pluggables>` and `<Plug>` components register
themselves with the Provider via React context.

Note: It is important to note that while Pluggables are a powerful framework for enhancing component integration, they are currently not compatible with server-side rendering (SSR). They excel in providing dynamic visual enhancements for CMSUI and client components/widgets. However, when it comes to handling critical data that needs to load quickly, alternative approaches may be more suitable.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use MyST syntax and one sentence per line for docs.

Suggested change
Note: It is important to note that while Pluggables are a powerful framework for enhancing component integration, they are currently not compatible with server-side rendering (SSR). They excel in providing dynamic visual enhancements for CMSUI and client components/widgets. However, when it comes to handling critical data that needs to load quickly, alternative approaches may be more suitable.
```{note}
While Pluggables are a powerful framework for enhancing component integration, they are currently not compatible with server-side rendering (SSR).
They excel in providing dynamic visual enhancements for the user interface, such as client components and widgets.
However, when it comes to handling critical data that needs to load quickly, alternative approaches may be more suitable.
```

@stevepiercy
Copy link
Collaborator

@Akshat2Jain this PR needs a change log entry: https://6.docs.plone.org/contributing/index.html#change-log-entry

You can ignore the failed docs build, which should be fixed in #4941

@stevepiercy
Copy link
Collaborator

@Akshat2Jain you need to merge the latest changes from master into this branch. There should be a button in the GitHub UI. The failing docs check will pass after you do so.

Screen Shot 2023-07-08 at 1 14 24 AM

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

A few quick and easy MyST fixes, and it is good to go!

FYI you can preview the built docs either by doing make docs-html or make docs-livehtml locally, or looking at it in the Netlify link in the pull request on GitHub. By checking the Netlify preview, you can see the problems.

Screen Shot 2023-07-08 at 1 20 27 AM

docs/source/recipes/pluggables.md Outdated Show resolved Hide resolved
docs/source/recipes/pluggables.md Outdated Show resolved Hide resolved
docs/source/recipes/pluggables.md Outdated Show resolved Hide resolved
docs/source/recipes/pluggables.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Docs changes look good. However, I just noticed that you should restore the deleted change log entry.

news/4932.bugfix

news/4932.bugfix Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Almost there! This tidies up the change log.

news/4932.bugfix Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Thank you! If it is all green, then it can be merged.

@stevepiercy
Copy link
Collaborator

@sneridagh this is docs only, and I approve. I'll let you merge and release.

@sneridagh sneridagh merged commit 2bf7c31 into plone:master Jul 16, 2023
44 checks passed
@Akshat2Jain Akshat2Jain deleted the pluggables branch July 16, 2023 17:33
sneridagh added a commit that referenced this pull request Jul 21, 2023
Co-authored-by: Akshat Jain <101265586+Akshat2Jain@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Pluggables are not SSR friendly
3 participants