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

Storybook property name inconsistencies #824

Open
6 of 16 tasks
rajsite opened this issue Nov 15, 2022 · 2 comments
Open
6 of 16 tasks

Storybook property name inconsistencies #824

rajsite opened this issue Nov 15, 2022 · 2 comments
Labels
documentation Improvements or additions to documentation

Comments

@rajsite
Copy link
Member

rajsite commented Nov 15, 2022

🐛 Bug Report

The property name documentation in storybook is inconsistent. For example see how in the following screenshot how some properties are kebab-case and some are camelCase.

💻 Repro or Code Sample

image

🤔 Expected Behavior

The storybook documentation should be consistent and make it clear when what is the difference between attributes, properties, events, slot configuration, generic storybook controls

😯 Current Behavior

See repro above.

💁 Possible Solution

Current proposal after discussing at standup:

Alternatives:

  • attributes:
    • Show the attribute names content-hidden
    • Show attribute names with the CSS selection syntax to emphasize it is an attribute [content-hidden]
  • For properties that do not have corresponding attribute
    • Prepend the name with a period .indeterminate
    • Use the prototype syntax common on mdn to emphasize it is a property Radio.prototype.indeterminate
  • For detailed topics that require a discussion and are not coupled to a property or attribute,
    • Topic: Start Icon, Discussion: Start Icon, wrap in quotes "Start Icon", just rely on sentence case Start Icon
  • Slots
    • Treat as topic Icon
    • Treat as part of the API: slot="start" or slot "start"
  • Events
    • Event close
  • Method
    • Method close, close(), or show full type signature: close(result: T): T | USER_DISMISSED

Another alternative:

  • Do not expose full API, only attributes and properties

🔦 Context

Current documentation can lead to confusion as discussed in this teams thread.

🌍 Your Environment

N/a

Remaining tasks

  • make it clear that the default slot isn't slot="default" and preferably show it as the first slot in the docs
  • consistent ordering of table categories in stories and in "Component APIs" docs (probably slots, attributes, properties, methods, events, and localizable labels) A way to sort categories in controls table storybookjs/storybook#27520
  • consistent language in docs
    • don't start descriptions with "This attribute..."
    • consistent way to mark attributes as required (*? text?)
    • events start with "Event emitted...". Maybe remove the word "Event"?
    • get rid of "A function that"
  • document using forms instead of binding to change events and value attributes
  • update incubating and spright components
  • Improve API documentation for input components #2132
  • API docs for additional components #2135
  • update table components
  • update banner
  • consistently document default values for attributes
  • consistently document the type name for enum attributes and see if we can link to their type nicely
  • Prettier not running on storybook files #2136
  • improve heading hierarchy for pages that document multiple components API docs for additional components #2135 (comment)
  • icon slot should use radio instead of boolean controls
  • consider hiding stories as children of mdx files (by moving to Internal)
  • capture more of these standards in CONTRIBUTING
@rajsite rajsite added bug Something isn't working triage New issue that needs to be reviewed labels Nov 15, 2022
@jattasNI jattasNI removed the triage New issue that needs to be reviewed label Nov 15, 2022
@rajsite
Copy link
Member Author

rajsite commented Nov 15, 2022

@nate-ni
Copy link
Contributor

nate-ni commented Nov 16, 2022

@rajsite , I think what we have today (direct controls), mixed with the grouping like Carbon is a great step forward. It also nice that Carbon provides that alternate syntax in the description, when appropriate (supports good ole copy/paste).

image

Looks like the grouping you mentioned from storybook would provide that as a solution for us.

🤔 Question

Does it have to be the same layout on both the "Controls" tab shown in the "Canvas" view AND the "Docs" view?

If so, no big deal. If not, might be nice to show the simplified (non-grouped) version in the Controls tab for simpler manipulation and the grouped version for the Docs view.

@nate-ni nate-ni added tech debt and removed bug Something isn't working labels Jan 18, 2023
@rajsite rajsite mentioned this issue Feb 8, 2023
1 task
@jattasNI jattasNI added documentation Improvements or additions to documentation and removed tech debt labels Mar 6, 2023
@jattasNI jattasNI self-assigned this Apr 16, 2024
fredvisser pushed a commit that referenced this issue Apr 24, 2024
# Pull Request

## 🤨 Rationale

As part of #824 we want to improve the accuracy and comprehensiveness of
our storybook API documentation.

## 👩‍💻 Implementation

Apply the following patterns to the menu button docs:

1. create sections in the args table for attributes, slots, and events.
(We'd also add methods for components that expose interesting ones)
2. Ensure the title and description are accurate for each member
3. link to shared docs about component APIs. 

Other minor bug fixes:
1. hide the "patterns" page from public storybook
2. hide text from ✅ icon mapping in component status table

### Possible follow up work

1. update doc templates and the docs for other existing components
similarly
1. more detailed API overview docs
- explanation of event patterns in web components and how they map to
frameworks
    - using forms instead of change events
1. see if we can remove the type annotation from control descriptions
2. [wait for further feedback] more curated examples and highlight most
important APIs for each component
1. [out of scope] I wish I could hide the Default column since we don't
bother populating it but that doesn't seem to be possible
storybookjs/storybook#15780


## 🧪 Testing

Manual storybook interaction

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this issue May 8, 2024
# Pull Request

## 🤨 Rationale

Continuing the work done in #2027 in service of #824. This PR covers the
menu component.

Example of why this is needed: after documenting the `nimble-menu-item`
`change` event I searched for usages in SLE and found dozens of
instances where apps were using the keyboard inaccessible `click` event
instead. I'm fixing them [in this
PR](https://dev.azure.com/ni/DevCentral/_git/Skyline/pullrequest/708606).

## 👩‍💻 Implementation

1. Refactored menu MDX and story files to follow patterns from previous
PRs. I made some judgement calls about where to document child
components vs examples; open to feedback on how it turned out!
1. Added more shared documentation strings and used them in several
existing stories.


## 🧪 Testing

Manual storybook inspection

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this issue May 17, 2024
# Pull Request

## 🤨 Rationale

As part of #824 we want to improve the accuracy and comprehensiveness of
our storybook API documentation.

## 👩‍💻 Implementation

1. Update `button`, `toggle-button`, and `card-button` to follow new API
doc pattern. (I'll do `anchor-button` along with other anchors).
2. Add and tweak wording on shared documentation strings and utilities
3. Tweak organization of MDX headings in all stories to put Appearance
and Sizing under Styling
4. Add CONTRIBUTING guidance about referring to sections within docs (in
previous PRs we gave up on trying to link to sections and started using
bolded text instead).

## 🧪 Testing

Local storybook exploration

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
jattasNI added a commit that referenced this issue May 21, 2024
# Pull Request

## 🤨 Rationale

As part of #824 we want to improve the accuracy and comprehensiveness of
our storybook API documentation. This PR covers:
- anchor
- anchor button
- anchor tab and related components
- tab and related components

## 👩‍💻 Implementation

1. Followed same patterns as previous PRs to restructure MDX files and
to add API categories to story files
2. Refactored anchor tab and tab to include separate stories for their
child components, similar to what I did for menu

## 🧪 Testing

Storybook exploration

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this issue May 24, 2024
# Pull Request

## 🤨 Rationale

Ongoing efforts towards #824. This PR covers input components:
- select
- combobox
- radio group
- text area
- text field
- number field
- checkbox
- switch

## 👩‍💻 Implementation

Generally follow patterns from previous PRs like #2126 and #2117. A few
interesting notes:
1. components have different behaviors regarding whether they sync their
`value` property to an attribute. I documented the behavior I observed
by adding a new table category for properties that don't have attributes
(also used by checkbox indeterminate property).
2. not yet documenting a recommendation to use form association / CVAs
instead of change events and value properties. Until I do so, the
`placeholder` docs for select are outside any table category.
3. added another new table category for localizable strings.
4. not yet documenting list option API in the select and combobox
stories. We (probably Meyer) are going to tackle this in a follow up
after #2111.

## 🧪 Testing

<!---
Detail the testing done to ensure this submission meets requirements. 

Include automated/manual test additions or modifications, testing done
on a local build, private CI run results, and additional testing not
covered by automatic pull request validation. If any functionality is
not covered by automated testing, provide justification.
-->

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: m-akinc <[email protected]>
jattasNI added a commit that referenced this issue May 28, 2024
# Pull Request

## 🤨 Rationale

Ongoing efforts towards #824. This PR builds on #2132 and covers these
components:
- tree
- toolbar
- icon
- dialog
- drawer
- spinner
- breadcrumb
- theme provider

## 👩‍💻 Implementation

Mostly following established patterns. Interesting bits:
- moved a `nimble-components/src/dialog/tests/types.ts` to the
`storybook` package since it iss only used by storybook
- collapsed 2 controls into 1 in the dialog story
- added a new table section called Styles for controls that impact CSS
- removed "allowNavigation" capability from breadcrumb story because it
was just showing native event behavior (i.e. how to cancel an event) and
didn't have any documentation
- moved "Target Configuration" section under the relevant component API
section for several components.

## 🧪 Testing

Storybook inspection

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this issue May 30, 2024
# Pull Request

## 🤨 Rationale

Ongoing efforts towards #824. This PR covers the table and its columns.

## 👩‍💻 Implementation

Mostly following established patterns. Some interesting notes:
1. For the Table Column Configuration doc, we had divided the
documentation into separate targeted stories. I decided these were
useful but we also should have a single table describing the whole API
of a column. I created the latter under the API section and moved the
targeted stories to an Examples section. Let me know if it feels right!
2. The above resulted in a renamed story which appears as a Chromatic
diff.

## 🧪 Testing

Interacting with the built Storybook.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this issue May 31, 2024
…ts (#2141)

# Pull Request

## 🤨 Rationale

Ongoing efforts towards #824. This PR
covers the remaining components: banner, incubating components, and
spright components.

After this completes all the components are done but there are still
some cross-component consistency improvements described in a checklist
on #824.

## 👩‍💻 Implementation

Following standard patterns from previous PRs: reorganizing MDX
headings, adding categories to each parameter in the args table, and
making language and naming more consistent.

For the rich text components I also migrated some content around to
better fit our standard document structure.

## 🧪 Testing

Inspected built storybook.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@jattasNI jattasNI mentioned this issue Jun 5, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Defined/Ready to Pickup
Development

No branches or pull requests

3 participants