Skip to content

Conversation

@nicolethoen
Copy link
Collaborator

@nicolethoen nicolethoen commented Jan 18, 2023

Closes #3354
Closes #3398

@nicolethoen nicolethoen requested a review from edonehoo January 18, 2023 14:54
@nicolethoen nicolethoen self-assigned this Jan 18, 2023
@nicolethoen nicolethoen removed their assignment Jan 18, 2023
@nicolethoen nicolethoen requested a review from evwilkin January 18, 2023 14:55
@patternfly-build
Copy link
Collaborator

patternfly-build commented Jan 18, 2023

@nicolethoen nicolethoen requested a review from mcarrano January 18, 2023 22:33
Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

@nicolethoen I guess I am OK with this as an interim fix. Where did the content you are linking to come from? Does that need to be reviewed at all? I agree that it makes sense to replace the broken training modules.

@nicolethoen
Copy link
Collaborator Author

the steps are borrowed from the existing, non-working katacoda tutorials. But since we don't have the live coding pane here that katacoda had, I created code sandboxes for people to use to follow the steps and a final solution code sandbox when appropriate.

I didnt remake any of the more complex trainings at this point. I tagged @edonehoo for a content review. I can ping some devs to try out the trainings, too.

@nicolethoen nicolethoen requested a review from srambach January 19, 2023 15:33
Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

I only looked at the HTML/CSS training modules. The code sandboxes work fine I think, but a few tweaks needed.

  • I missed the sandbox link at first (knowing it was there). Could it be a button to make it more prominent?
  • Could the places where you are supposed to copy the code have a button to automatically copy it?
  • CSS variables and alerts, codesandbox in part 1 - I don't see an alert in it.
  • // is not a valid comment delimiter for .css files, so if that's left in the file then nothing works. You can use /* */ instead.

@nicolethoen nicolethoen requested a review from kmcfaul January 23, 2023 20:16
@kmcfaul
Copy link
Contributor

kmcfaul commented Jan 24, 2023

Agreed with @srambach that the codesandbox link (for the react example) can get a bit lost in the rest of the figure diagrams. Maybe we can put the link in a kind of "Try it yourself" card? It may also be worth mentioning that the imports are being done for you in the codesandbox. I was initially a bit confused about the unused imports until going through the rest of the tutorial.

Edit: I see you updated the codesandbox to be a button, which is better but I think a card may look better. Not sure, I'm happier with the button over the inline link still. The 3 figures in Step 1 look a little busy to me but I'm not sure how to break it up (maybe we break up the paragraph that references them, bulk out the content describing each image and place the paragraphs between the pictures?)

Steps 2.2 and 3.1 list variants of a component but in different ways. We should make this uniform I think, either as plain text, or in quotes, or with the code markup. Step 3.1 also doesn't introduce TextVariants as a type interface before using it so a little blurb there would be helpful maybe.

@kmcfaul
Copy link
Contributor

kmcfaul commented Jan 24, 2023

It might also look nice to have a divider between each Step to block out each section a bit more but that's minor and I'm happy with or without it.

Copy link
Collaborator

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

This level of review is maybe too slow/nitpicky/detailed for the number of pending changes. Didn't realize that until now, so I'm going ahead and sending through the suggestions I made (only started looking at the html/css training), but I can look at the rest more generally going forward for the sake of time and not holding you back too long (if you'd prefer)!

@nicolethoen
Copy link
Collaborator Author

@edonehoo keep them coming!

@srambach
Copy link
Member

Edit: I see you updated the codesandbox to be a button, which is better but I think a card may look better. Not sure, I'm happier with the button over the inline link still. The 3 figures in Step 1 look a little busy to me but I'm not sure how to break it

IMHO it should be a button. I didn't initially see the card as a clickable thing, and buttons are much more of a "take action here" element. I also like the suggestion to use wording something like "Try out components yourself in a Codesandbox" (I like the simplicity of "Try it yourself" but that isn't the best for a11y because there would be multiples.

Copy link
Collaborator

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

finished a quick pass through the content & made some tweaks here and there!

Copy link
Member

@evwilkin evwilkin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@maryshak1996 maryshak1996 left a comment

Choose a reason for hiding this comment

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

Assuming that the scope of this is just to fix things that are broken, this looks good to me!

@evwilkin evwilkin merged commit a8dee99 into patternfly:main Feb 27, 2023
nicolethoen added a commit that referenced this pull request Apr 10, 2023
* changes to tooltip guidelines (#3400)

* changes to tooltip guidelines

* added indentation for images to be under each bullet

---------

Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.11.1
 - @patternfly/documentation-framework@1.6.1
 - patternfly-org-4@4.17.1

* feat(docs): customize tabs with optional tabText frontmatter (#3394)

* feat(docs): customize tabs with optional tabText frontmatter

* improved variable names

* updated examples to document tabName

* PR feedback - let to const

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0
 - @patternfly/documentation-framework@1.7.0
 - patternfly-org-4@4.18.0

* feat(docs): enabled manual ordering of sidenav (#3403)

* feat(docs): enabled manual ordering of sidenav

* cleanup & working

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.13.0
 - @patternfly/documentation-framework@1.8.0
 - patternfly-org-4@4.19.0

* fix(whitespace): test github actions

* add action guidance (#3347)

* add action guidance

* more additions

* action updates

* update last example image

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.13.1
 - @patternfly/documentation-framework@1.8.1
 - patternfly-org-4@4.19.1

* feat(trainings): remove broken trainings and replace some with blog style training (#3353)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.0
 - @patternfly/documentation-framework@1.9.0
 - patternfly-org-4@4.20.0

* docs(Badge): Small typo in a11y section (#3405)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.1
 - @patternfly/documentation-framework@1.9.1
 - patternfly-org-4@4.20.1

* Updates colors guideline content. (#3233)

* Updates status colors to match alert component recommendations. Also makes small edits across file.

* Adjust grid item.

* Adds disabled color swatches and makes minor content changes.

* Quick update to status and state color description.

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.2
 - @patternfly/documentation-framework@1.9.2
 - patternfly-org-4@4.20.2

* feat(docs): add react console documentation (#3366)

* feat(docs): add react console documentation

* fix(docs): add loader for jsx to extensions

* chore(docs): bump react console version

* chore(deps): bump console version

* chore(deps): bump console version

* fix(deps): re-added topology dependency

* chore(console): bump console version and generate screenshot

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.0
 - @patternfly/documentation-framework@1.10.0
 - patternfly-org-4@4.21.0

* chore: remove hard coded GA ID (#3431)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.1
 - @patternfly/documentation-framework@1.10.1
 - patternfly-org-4@4.21.1

* fix(menu): update and clarify description (#3438)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.2
 - @patternfly/documentation-framework@1.10.2
 - patternfly-org-4@4.21.2

* updates to skeleton guidelines (#3422)

* updates to skeleton guidelines

* sentence and grammar edits

* Trying again to stage and push

* Trying again to stage and push

* trying again to stage

* trying again to stage

* Update skeleton.md

* updated text but still need images

* Added centered files

* added images

* fixed everything from comments

* Update spinner.md

---------

Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.3
 - @patternfly/documentation-framework@1.10.3
 - patternfly-org-4@4.21.3

* fix(release): update docs for 2023.02 release (#3453)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.4
 - @patternfly/documentation-framework@1.10.4
 - patternfly-org-4@4.21.4

* Added guidance around disabling menu actions (#3424)

* Added guidance around disabling menu actions

* Updated disabled option guidelines

* Added example image for disabled actions

* Updated formatting of disabled menu guidance section

* Formatting of disabled menu section

* Adjusted image sizes

* Update menu.md

---------

Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>

* feat(v5): pull in alphas, set up doc-framework alpha (#3401)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.0
 - @patternfly/documentation-framework@2.0.0-alpha.1

* fix(whitespace): test github actions

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.1
 - @patternfly/documentation-framework@2.0.0-alpha.2

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.2
 - @patternfly/documentation-framework@2.0.0-alpha.3

* fix(README): small wording change to test build

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.3
 - @patternfly/documentation-framework@2.0.0-alpha.4

* fix(404 Page): Removed CardHeaderMain (#3429)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.4
 - @patternfly/documentation-framework@2.0.0-alpha.5

* chore(Card): update a11y card docs (#3384)

* update a11y card docs

* PR feedback

Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com>

* format

---------

Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com>

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.5
 - @patternfly/documentation-framework@2.0.0-alpha.6

* chore(multiple): updated verbiage for arialabel props or attributes (#3396)

* chore(multiple): updated verbiage for arialabel props or attributes

* Updated anchors to Link component

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.6
 - @patternfly/documentation-framework@2.0.0-alpha.7

* feat(docs-framework): add legacy ssl cli option (#3433)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.7
 - @patternfly/documentation-framework@2.0.0-alpha.8

* fix(versions): add alpha.0 of drag-drop

* fix(versions): fix typo

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.8
 - @patternfly/documentation-framework@2.0.0-alpha.9

* Update to website title PatternFly 5 (#3443)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.9
 - @patternfly/documentation-framework@2.0.0-alpha.10

* Update website IA for v5 (#3416)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.10
 - @patternfly/documentation-framework@2.0.0-alpha.11

* updated community with yew (#3461)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.11
 - @patternfly/documentation-framework@2.0.0-alpha.12

* chore(webpack5): Initial working commit with webpack5 (#3454)

* chore(webpack5): Initial working commit with dev server and build working so far for webpack 5 update

* chore(monaco): Update monaco plugin and dependent version

* fix ssr

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.12
 - @patternfly/documentation-framework@2.0.0-alpha.13

* fix(Tables): temporarily comment out tables so Table can be deprecated (#3468)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.13
 - @patternfly/documentation-framework@2.0.0-alpha.14

* Iss3159 (#3442)

* fix(Tabs): update tabs guidelines

* Update tabs.md

* Update tabs.md

---------

Co-authored-by: nicolethoen <nthoen@redhat.com>
Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0-alpha.14
 - @patternfly/documentation-framework@2.0.0-alpha.15

* rebased from main

---------

Co-authored-by: Tina Yip <98424339+tiyiprh@users.noreply.github.com>
Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>
Co-authored-by: patternfly-build <patternfly-build@redhat.com>
Co-authored-by: Nicole Thoen <nthoen@redhat.com>
Co-authored-by: E Gustavsson <eric@spytec.se>
Co-authored-by: edonehoo <105813956+edonehoo@users.noreply.github.com>
Co-authored-by: Austin Sullivan <ausulliv@redhat.com>
Co-authored-by: kelseamann <91503095+kelseamann@users.noreply.github.com>
Co-authored-by: Katie Edwards <94569315+kaedward@users.noreply.github.com>
Co-authored-by: Titani Labaj <39532947+tlabaj@users.noreply.github.com>
Co-authored-by: Jenny <32821331+jenny-s51@users.noreply.github.com>
Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com>
Co-authored-by: mcarrano <mcarrano@redhat.com>
Co-authored-by: Dana Gutride <dgutride@gmail.com>
Co-authored-by: kuklas <78739379+kuklas@users.noreply.github.com>
nicolethoen added a commit that referenced this pull request Apr 27, 2023
* feat(docs): added component description to header above tabs

* feat(docs): added component description to header above tabs

* set up components-info.json, pass to webpack.base.config

* pass componentInfo to example.js

* working except for Links in mdx to avoid anchor tags

* update naming - image to illustration

* revert to dangerouslySetInnerHTML

* convert string to JSX to maintain router links

* remove word doc

* reapply tile fix

* clean up logic to avoid undefined errors

* fix form select & multiple file upload summaries

* sort View all components to top of components sidenav

* added component gallery

* pull in component summary, illustration link

* illustrations working, moved to v4

* insert toolbar & item count

* feat(docs): add component gallery

* increase toolbar width to offset pulling it left

* add optional chaining and default values for drawerPanelData

* add reset button in place of searchinput clear button

* design feedback & css styling

* responsive

* remove unused code, fix sticky top on mobile

* PR feedback 1

* gallery container sizing and spacing, default image

* coker feedback, refactor toolbar & fix search interaction

* fix hide sidebar based on search results

* persistent sidebar

* add list view

* remove sidebar, add beta labels, click to navigate

* data list max-width

* fix toolbar z-index, refactor duplicate code

* fix duplicate summary due to incorrect rebase

* Fix surge links?

* moving code into doc-framework

* add alias for v4 component images in doc-framework webpack base config

* remove link styling, nested links, beta labels on next components

* adding hasOverview field to trigger gallery view

* changes to tooltip guidelines (#3400)

* changes to tooltip guidelines

* added indentation for images to be under each bullet

---------

Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.11.1
 - @patternfly/documentation-framework@1.6.1
 - patternfly-org-4@4.17.1

* feat(docs): customize tabs with optional tabText frontmatter (#3394)

* feat(docs): customize tabs with optional tabText frontmatter

* improved variable names

* updated examples to document tabName

* PR feedback - let to const

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.12.0
 - @patternfly/documentation-framework@1.7.0
 - patternfly-org-4@4.18.0

* feat(docs): enabled manual ordering of sidenav (#3403)

* feat(docs): enabled manual ordering of sidenav

* cleanup & working

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.13.0
 - @patternfly/documentation-framework@1.8.0
 - patternfly-org-4@4.19.0

* fix(whitespace): test github actions

* add action guidance (#3347)

* add action guidance

* more additions

* action updates

* update last example image

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.13.1
 - @patternfly/documentation-framework@1.8.1
 - patternfly-org-4@4.19.1

* feat(trainings): remove broken trainings and replace some with blog style training (#3353)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.0
 - @patternfly/documentation-framework@1.9.0
 - patternfly-org-4@4.20.0

* docs(Badge): Small typo in a11y section (#3405)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.1
 - @patternfly/documentation-framework@1.9.1
 - patternfly-org-4@4.20.1

* Updates colors guideline content. (#3233)

* Updates status colors to match alert component recommendations. Also makes small edits across file.

* Adjust grid item.

* Adds disabled color swatches and makes minor content changes.

* Quick update to status and state color description.

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.14.2
 - @patternfly/documentation-framework@1.9.2
 - patternfly-org-4@4.20.2

* feat(docs): add react console documentation (#3366)

* feat(docs): add react console documentation

* fix(docs): add loader for jsx to extensions

* chore(docs): bump react console version

* chore(deps): bump console version

* chore(deps): bump console version

* fix(deps): re-added topology dependency

* chore(console): bump console version and generate screenshot

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.0
 - @patternfly/documentation-framework@1.10.0
 - patternfly-org-4@4.21.0

* chore: remove hard coded GA ID (#3431)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.1
 - @patternfly/documentation-framework@1.10.1
 - patternfly-org-4@4.21.1

* fix(menu): update and clarify description (#3438)

* chore(release): releasing packages [ci skip]

 - @patternfly/ast-helpers@0.15.2
 - @patternfly/documentation-framework@1.10.2
 - patternfly-org-4@4.21.2

* fix search params, remove hasOverview code

* Move page .md from docs-framework to v4

* fixed placement of beta labels

* split gallery page into composable components

* added customizable text, documented params

* fixed toolbar width

* moved .md file to pages directory

* move gallery page, remove hideSourceTabs

* updated to enable toggling of subsection parsing

* update & comment CSS

* updates per comments

---------

Co-authored-by: Tina Yip <98424339+tiyiprh@users.noreply.github.com>
Co-authored-by: Margot <51165119+mmenestr@users.noreply.github.com>
Co-authored-by: patternfly-build <patternfly-build@redhat.com>
Co-authored-by: Nicole Thoen <nthoen@redhat.com>
Co-authored-by: E Gustavsson <eric@spytec.se>
Co-authored-by: edonehoo <105813956+edonehoo@users.noreply.github.com>
Co-authored-by: Austin Sullivan <ausulliv@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - Broken link for PF design kit training Clean up old katacoda trainings and replace them

8 participants