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

Updated documentation for Typography #2324

Merged
merged 65 commits into from
Jan 6, 2025
Merged

Updated documentation for Typography #2324

merged 65 commits into from
Jan 6, 2025

Conversation

smmr-dn
Copy link
Contributor

@smmr-dn smmr-dn commented Nov 4, 2024

Changes

Updated existed documentation for Typography.

Testing

N/A

Docs

N/A

After-PR TODO:

@smmr-dn smmr-dn marked this pull request as ready for review December 16, 2024 14:01
@smmr-dn smmr-dn requested a review from a team as a code owner December 16, 2024 14:01
@smmr-dn smmr-dn requested review from mayank99 and r100-stack and removed request for a team December 16, 2024 14:01
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
examples/Text.muted.jsx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I think this example can be similar to the Label inline react story. Because, Flex further adds its gap.

Also, since this example is shown next to the list of display styles, I think we have to show an example of "block" too. The file can be renamed accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the "block" style is default which is shown in the main example. Should I move it down?

Copy link
Member

@r100-stack r100-stack Dec 23, 2024

Choose a reason for hiding this comment

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

Not sure what you meant by "move it down". But what I meant to say was that this example can also include a label + input pair with "block" to show the difference between block and inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh what I meant was to move the main example down to the Display styles section. I created a LabelSelect so that users can switch between the two layouts.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of switching between the two, I think we can have two separate Label + Input pairs within the same example, one "block" and the other "inline". That way users can see both simultaneously.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have combined two labels into one.

apps/website/src/content/docs/typography.mdx Outdated Show resolved Hide resolved
Copy link
Member

@r100-stack r100-stack left a comment

Choose a reason for hiding this comment

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

LGTM after all existing comments are marked as resolved 👍

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM after #2324 (comment)

@smmr-dn smmr-dn mentioned this pull request Jan 6, 2025
2 tasks
@smmr-dn smmr-dn merged commit d241fc2 into main Jan 6, 2025
17 of 18 checks passed
@smmr-dn smmr-dn deleted the uyen/typography-doc branch January 6, 2025 22:19
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.

3 participants