-
Notifications
You must be signed in to change notification settings - Fork 723
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
[WTF-1907] Fix and improve how-to guide for building pluggable web widgets #8044
[WTF-1907] Fix and improve how-to guide for building pluggable web widgets #8044
Conversation
Thanks @legio-vi-ferrata ! Is there a tester you recommend if I have questions or get error codes? |
Hello @ConnorLand, are you intending to run through the guide to test it? That would be awesome to be honest. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass of my review. So far I have checked the index and part 1. I will probably get to part 2 tomorrow.
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished my pass on Part 2
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @weirdwater, a lot of nice catches! I addressed all threads. Can you do a re-review, please?
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-one.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
…accessibility section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weirdwater, ready for re-re-review.
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
48a4416
to
4e46b72
Compare
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@legio-vi-ferrata I have "resolved" some of the threads, and provided a suggestion for the cryptic code description for the accessibility section. I also found some code snippet improvements along the way 🙈
Sorry, some of the comments were posted directly and one was stuck in concept-review...
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the final review is done now. :)
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Outdated
Show resolved
Hide resolved
content/en/docs/howto/extensibility/pluggable-widgets/create-a-pluggable-widget-two.md
Show resolved
Hide resolved
@ConnorLand, we have finished our internal review process. This one is all yours. |
@legio-vi-ferrata just a quick word about testing. @weirdwater and I discussed it. Because the product changes for the widget generator are live, it's key to merge these docs ASAP. I've edited them for language and some formatting and will merge the PR presently. I will do a full test of the docs myself and see if they require any tweaks to the instructions. I hope to finish that task this week. Thankfully, Arjo did some nice testing himself and the changes are almost all code-snippet improvements (thus, they don't break the doc's language/instructions). |
We have previously refactored the Part 1 of the guide for building pluggable web widgets. This PR is a follow-up story to upgrade Part 2 with the latest information and conventions as well. This has become necessary as the first part now uses functional React components in code examples with which following the Part 2 is not possible.
In addition, the PR contains several other improvements for both Part 1 and Part 2.
Note to Reviewer/Tester: I would heavily recommend running the dev server of the repository and following the steps at least once. This would also probably make it easier to see the differences among example code snippets.
Release Information: The PR can be merged and released as soon as possible after its draft status is lifted.