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

Reset and document Prose component styles #390

Merged
merged 2 commits into from
Jan 26, 2024
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Dec 19, 2023

🛠 Summary of changes

Removes override styles for the Prose component and adds a preview page to the internal documentation.

Related Slack discussion: https://gsa-tts.slack.com/archives/C01JVKYE4KD/p1702914619083849

Related LGDS Office Hours discussion notes: https://docs.google.com/document/d/10b7Gis_VxYT3roR01MX4hwPa6-Tq29YBaLPIB3vWnUw/edit#bookmark=id.w558e3npl6ym

Live preview URL: https://federalist-340d8801-aa16-4df5-ad22-b1e3e731098e.sites.pages.cloud.gov/preview/18f/identity-design-system/aduth-typography-prose/prose/

(Draft pending audit of effect of removing overrides)

👀 Screenshots

federalist-340d8801-aa16-4df5-ad22-b1e3e731098e sites pages cloud gov_preview_18f_identity-design-system_aduth-typography-prose_prose_(Computer Standard)

@aduth aduth requested a review from nickttng December 19, 2023 14:53
@aduth aduth force-pushed the aduth-typography-prose branch from dd6d5b8 to 18a9405 Compare January 4, 2024 13:48
Copy link
Member

@nickttng nickttng left a comment

Choose a reason for hiding this comment

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

The Prose styling looks good to me - Nothing that appears outstanding to flag.

Thinking about the next steps

  • Create a "Prose" typography section in LGDS Figma
    • Brief guidance for "Prose" vs. default, i.e., Prose for static sites
  • When this PR is approved, announce that Prose is available, and used for static sites

Anything in addition?

@aduth
Copy link
Member Author

aduth commented Jan 8, 2024

I recall we'd implemented custom styles for the brochure site since we wanted specific text styling that wasn't reflected by Prose at the time. Do we think we'd need a review step to reconcile potential changes if we were to introduce the prose style reset to the brochure site, or should we just assume that Prose is correct as-is, even if it results in some spacing deviations from the current brochure site?

As far as specifics of rolling this out, this might be one we'd consider to be a "Breaking Change" which doesn't mean whole lot in terms of potential impact, but it could be wise for us (me) to ship the current unreleased changes in a minor release, so we could offer an incremental upgrade path for downstream projects. There's at least one other breaking change I have planned (dropping support for IE11), so it could be good timing to pair with that.

@nickttng
Copy link
Member

nickttng commented Jan 9, 2024

Do we think we'd need a review step to reconcile potential changes if we were to introduce the prose style reset to the brochure site, or should we just assume that Prose is correct as-is, even if it results in some spacing deviations from the current brochure site?

It'd be good to have a review step for pages with running text, like Help Center articles. Other brochure site pages can be a lightweight visual check.

For Developers and Partners, do we want to notify Team Ursula (?) that Prose is available if they want to implement it, if not already?

There's at least one #387 (comment), so it could be good timing to pair with that.

This sounds like a good pairing.

@aduth
Copy link
Member Author

aduth commented Jan 9, 2024

Ok, what I might do is this:

  1. I publish 8.1.0 release with the current unreleased changes Edit: Done ✅
  2. I merge this pull request Edit: Done ✅
  3. I publish a beta prerelease version 9.0.0-beta.1 Edit: Done ✅
  4. I create a pull request in identity-site which updates to the new version and removes all prose styling overrides Edit: Done
  5. We collectively review the resulting preview branch to check for regressions
  6. If changes are necessary in identity-design-system, we make and merge those changes
  7. Repeat until we're comfortable that the styles are settled
  8. (Somewhere in parallel to all of what's going on between step 3 and 7, I complete and merge the revival of Replace Babel with ESBuild #387 (comment) ) Edit: Done ✅
  9. I publish 9.0.0 final version including (1) Prose stye changes and (2) changes mentioned in Replace Babel with ESBuild #387 (comment)

For Developers and Partners, do we want to notify Team Ursula (?) that Prose is available if they want to implement it, if not already?

I think a review process using beta releases like described in the list above would make sense for the developer site, sure 👍 I might take that on myself to initiate and coordinate review with the partnerships teams.

@nickttng
Copy link
Member

nickttng commented Jan 9, 2024

Smells like a plan! 👃🏼

@aduth aduth mentioned this pull request Jan 23, 2024
@aduth aduth added the breaking change Backwards-incompatible change to be included in a major version release label Jan 23, 2024
@aduth aduth marked this pull request as ready for review January 26, 2024 15:29
@aduth aduth force-pushed the aduth-typography-prose branch from 18a9405 to e2226a2 Compare January 26, 2024 15:33
@aduth aduth force-pushed the aduth-typography-prose branch from e2226a2 to a2570c6 Compare January 26, 2024 15:36
@aduth
Copy link
Member Author

aduth commented Jan 26, 2024

Visual regression failures are expected since (1) there's a new sidebar navigation link for Prose documentation and (2) the internal documentation site itself uses Prose, which would be affected by these styling changes.

@aduth aduth merged commit 5253373 into main Jan 26, 2024
1 of 2 checks passed
@aduth aduth deleted the aduth-typography-prose branch January 26, 2024 15:38
@aduth
Copy link
Member Author

aduth commented Jan 26, 2024

  1. I create a pull request in identity-site which updates to the new version and removes all prose styling overrides

All steps up to this are complete, and the preview pull request is at GSA-TTS/identity-site#1207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Backwards-incompatible change to be included in a major version release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants