Component documentation feedback

bhoppers2008 commented 1 year ago

A place to capture feedback on the docs template or the site page itself. Where are the challenges? Is there anything missing? Is it difficult to find a place for content you want to add? Etc...

bhoppers2008 commented 1 year ago

[x] 1. Update token in preview area to --container-primary-background token ( @JoanaMMoreira )
[ ] 2. Standardise the 'tip' or guidance per feature (i.e., example) so it's consistently displayed the same way (e.g., in a call-out).
[ ] 3. Longer term, have an iframe with the Figma component guide up on the site - short term include this in the 'Getting started - Designing with Salt' page.
[ ] 4. Include more specific design guidance on the site docs in the near future.
[x] 5. LivePreview bug (@james-nash PR merged)
[ ] 6. The props do not have enough affordance on the page, they currently blend into the rest of the text. Let's aim to enhance this post docs sprint so they are clearer.
[x] 7. Source code preview (which uses react-element-to-jsx-string) doesn't display the component name but <No Display Name> in some scenarios e.g. BorderLayout, BorderItem and FormField. See parseReactElement.js#L49-L76. Consider mosaic sandpack issue. Fixed by #2349.
[ ] 8. Provide useful documentation solution for controlled examples, rather than bundling them into the list of feature examples.
[ ] 9. Clicking on the chevron in the side nav should only expand/collapse the item, not open the page at the same time.
[x] 10. Similar to 7, some complex example would have bundled code, e.g. in Toast / ToastGroupBottom example code: /*#__PURE__*/ (0,react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__.jsx)(_ToastsExamples__WEBPACK_IMPORTED_MODULE_2__.InfoToast, {})}].concat((0,_swc_helpers_to_consumable_array__WEBPACK_IMPORTED_MODULE_4__._)(toasts)));}}> or, Border Layout change radio doesn't change code. (@origami-z) This could be the result of transform code, where we just need to show original code, where also benefits users to copy paste. Consider mosaic sandpack issue. Fixed by #2349.
[ ] 11. Vertical alignment of "All examples" label and switch https://github.com/jpmorganchase/salt-ds/issues/2372
[x] 12. "Show code" switch doesn't persist when switching between examples - Issue created #2314 and assigned to @james-nash
[ ] 13. Some pills in "Also known as" "Similar to" should be read-only without a link (depends on #1873)
[ ] 14. Top nav overflows at some breaks points, e.g. 967px, where "Support and Contribution" is out of bound
[ ] 15. "Accessibility" tab's "Keyboard interactions" table content vertical alignment between "Key" and "Function" columns
[x] 16. SaltProvider_SaltProvider in show code. Fixed by #2349.
[ ] 17. "On this page" vertical alignment between bar and text
[ ] 18. Activation bar (orange underline) on Tabs disappears occasionally, need to upgrade the Tabs component to use the latest and check if this is still an issue. Similar bug happens to right hand side vertical nav "On this page" indicator bar.
[ ] 19. Overflowed item of "Select an example", should there be a tooltip? Alternative title?
[x] 20. Prop true/false wording consistency, a="true" v.s. b={true} https://github.com/jpmorganchase/salt-ds/pull/2330
[ ] 21. Surprised to see the site content is deployed when merged to main (with unreleased code/component), but storybook only deploys released code. Expecting it to be the other way around.
[ ] 22. "Select an example" list will reset and break (bigger bottom margin) when it overflows and select an item after 1st page, e.g. "Validation" on "Form Field"
[ ] 23. Consider adding a "related" relationship (in addition to "alsoKnownAs" and "contains") and surfacing that in right-hand nav (as per this PR discussion)
[ ] 24. Clarify intended use of "contains" relationship - should components that may be composed (e.g. icon inside a button) be included, or only things used directly by the component itsef? Also, should it be an exhaustive list and, if not, which contained components should be listed and which not?
[ ] 25. Would be cool if relationship links in the right-hand column were automatically bi-directional. E.g. if component A lists component B as one it is similar to, then a link back to component A should automatically appear on component B's page, even though it's not explicitly listed it as one of its similar to ones.
[ ] 26. Inconsistency of Props Table "default" column, where some defaults only mentioned in "Description" column, others have both.
[ ] 27. How to share code between storybook and site examples?
[ ] 28. Some component's initialVersion needs to be re-checked. It's also not revealed on site?
[ ] 29. Can the placement and position of the 'example' block be consistent across pages on the component docs? i.e. in the accessibility tab for Input, it sits underneath 2 bullets, which doesn't look great when users are flicking between the tabs.. https://www.saltdesignsystem.com/salt/components/input/accessibility
[ ] 30. Similar to 18, the active component in the left hand sidebars orange bar should stay present, but disappears after a few seconds.
[ ] 31. The headings in the 'how to use' section are inconsistent across the pages where we do not have a title before the 'when to use/ when not to use' guidance.. i.e. in Input vs Link. Notice the H3 vs H4 header change. Let's fix this in the template so that it is consistent across all pages.
[ ] 32. In code, frontmatter title has mixed usages. Only index.mdx should has string value, the other 3 mdx files should be $ref

el-dav commented 1 year ago

Note the displayName issue (number 7) seems to be due to the components using arrow functions rather than named functions, so we should create a ticket to go though and update all components to use named functions. Updating FlowLayout to the following fixed that component for me:

export const FlowLayout: FlowLayoutComponent = forwardRef(function FlowLayout<
  T extends ElementType = "div"
>({ children, ...rest }: FlowLayoutProps<T>, ref?: PolymorphicRef<T>) {
  return (
    <FlexLayout direction="row" ref={ref} wrap {...rest}>
      {children}
    </FlexLayout>
  );
});

from

export const FlowLayout: FlowLayoutComponent = forwardRef(
  <T extends ElementType = "div">(
    { children, ...rest }: FlowLayoutProps<T>,
    ref?: PolymorphicRef<T>
  ) => {
    return (
      <FlexLayout direction="row" ref={ref} wrap {...rest}>
        {children}
      </FlexLayout>
    );
  }
);

james-nash commented 1 year ago

Clicking on the chevron in the side nav should only expand/collapse the item, not open the page at the same time.

I believe this'll be fixed when we replace the current vertical nav component (which comes from Mosaic) with our own, based on the NavigationItem component.

Not sure there's much point trying to "fix" this in the Mosaic component now, given that we're due to replace it anyway.

james-nash commented 1 year ago

FYI, I've opened an issue to note some additional functionality for the LivePreview component (the thing that displays our component examples) that has been discussed in the past but never actually written down: #2348

jpmorganchase / salt-ds

Component documentation feedback #2265