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

Ab/document tokens #1332

Merged
merged 3 commits into from
Feb 9, 2022
Merged

Ab/document tokens #1332

merged 3 commits into from
Feb 9, 2022

Conversation

andrewberg-okta
Copy link
Contributor

This PR adds documentation for theming, generated from theme source files, to storybook.

The Guidelines/Design tokens page contains tables with all of the available theme tokens grouped by type
image

Each component documentation page now contains a table with its theme variables and their default token values
image

@andrewberg-okta andrewberg-okta requested a review from a team as a code owner February 8, 2022 23:29
Comment on lines 1 to 27
import { Canvas, Story, ArgsTable } from "@storybook/addon-docs";
import { theme } from "@okta/odyssey-react/dist/components/TextArea/TextArea.theme";
import { ThemeTable } from "../_internal/ThemeTable";

# TextArea

TextArea allows users to edit and input data.

<Canvas>
<Story id="components-textarea--default" />
</Canvas>

## Props

<ArgsTable />

<ThemeTable componentThemeReducer={theme} />

## Stories

<Canvas>
<Story id="components-textarea--disabled" />
</Canvas>

<Canvas>
<Story id="components-textarea--optional" />
</Canvas>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding these missing pages!

Comment on lines 1 to 13
/*!
* Copyright (c) 2022-present, Okta, Inc. and/or its affiliates. All rights reserved.
* The Okta software accompanied by this notice is provided pursuant to the Apache License, Version 2.0 (the "License.")
*
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0.
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
*
* See the License for the specific language governing permissions and limitations under the License.
*/

/* eslint-disable import/no-extraneous-dependencies */
Copy link
Contributor

Choose a reason for hiding this comment

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

In the past I have put these kinds of internal components in packages/odyssey-storybook/.storybook/components

I would like for us to stay consistent, but have no preference as to where these things should live. Will defer to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved in 3edfdbe
Feels better to have them out of src

<Table.DataCell>
<Text as="code">{componentVariables[variable]}</Text>
</Table.DataCell>
<Table.DataCell format={"num"}>
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick

Suggested change
<Table.DataCell format={"num"}>
<Table.DataCell format="num">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 3edfdbe

<Table.Row>
<Table.HeaderCell scope="col">Variable Name</Table.HeaderCell>
<Table.HeaderCell scope="col">Token Value</Table.HeaderCell>
<Table.HeaderCell scope="col" format={"num"}></Table.HeaderCell>
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick

Suggested change
<Table.HeaderCell scope="col" format={"num"}></Table.HeaderCell>
<Table.HeaderCell scope="col" format="num"></Table.HeaderCell>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 3edfdbe

Comment on lines 169 to 176
<span
style={{
width: "1.5em",
height: "1.5em",
display: "inline-block",
boxShadow: `${val}`,
}}
></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why not use a Box here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my initial stab, but the values that we get from the theme are absolute and the props that box wants aren't. So there's not much utility that box offers in this case. eg: here val = "0 2px 0 rgba(175, 175, 175, 0.12)" but box wants boxShadow = "default"

</tbody>
</table>
</figure>
<TokenTables />
Copy link
Contributor

Choose a reason for hiding this comment

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

📈 Nice.

@andrewberg-okta andrewberg-okta merged commit 61df89b into develop Feb 9, 2022
@andrewberg-okta andrewberg-okta deleted the ab/document-tokens branch February 9, 2022 22:17
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.

2 participants