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

Updates the course about page CSS #1865

Merged
merged 5 commits into from
Sep 7, 2023

Conversation

jkachel
Copy link
Contributor

@jkachel jkachel commented Sep 6, 2023

What are the relevant tickets?

#1862

Description (What does it do?)

This wraps the feature-flagged class changes (and just those changes) in a selector that should only apply if the feature flag is tagged on.

How can this be tested?

Set the feature flag to Enabled and check - it should look the same as it did in #1816 .

Set the feature flag to Disabled and check - it should successfully revert to the old design.

- Don't conditionally apply styles. Instead, always include the v2 styles (in product-page/).
- New styles are wrapped to only take effect if the body tag has the `new-design` class (which does get toggled by the feature flag)
- Started on removing duplication but will take a bit

The plan is to just leave the new design specific stuff wrapped in the body.new-design and then when it's time to eliminate the flag, pull that wrapper then use a linter or something to find the classes that aren't being used anymore.
@JenniWhitman JenniWhitman self-requested a review September 7, 2023 13:17
@JenniWhitman JenniWhitman self-assigned this Sep 7, 2023
Copy link
Contributor

@JenniWhitman JenniWhitman left a comment

Choose a reason for hiding this comment

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

Looks good - matches with current RC, doesn't modify other pages, and pulls us out of the business of loading extra stuffs.

@jkachel jkachel merged commit 4861916 into main Sep 7, 2023
@jkachel jkachel deleted the jkachel/1862-redo-course-about-classes branch September 7, 2023 14:02
@odlbot odlbot mentioned this pull request Sep 7, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants