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

DOP-4660: Applies dark mode to the Vertical Procedures and Steps Component #1101

Merged
merged 16 commits into from
May 30, 2024

Conversation

caesarbell
Copy link
Collaborator

Stories/Links:

DOP-4660

Current Behavior:

Atlas: Get Started
Click on the Atlas UI tab and verify the Vertical Procedures Component

Atlas: Connect to your Atlas Cluster
Verify the Steps Component

Staging Links:

Staged Atlas: Get Started
Click on the Atlas UI tab and verify the Vertical Procedures Component

Staged Altas: Connect to your Atlas Cluster
Verify the Steps Component

Notes:

  • Dark mode is enabled for both the Vertical Procedures Component and the Steps Component.

README updates

    • This PR introduces changes that should be reflected in the README, and I have made those updates.
    • This PR does not introduce changes that should be reflected in the README

Copy link
Collaborator

@mmeigs mmeigs left a comment

Choose a reason for hiding this comment

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

This looks pretty good.
I do think we all should probably decide on our preferred approach.

Do we use the context in DocumentBody and pass the prop through everything? Or should we use the context in each (parent) component and only pass down if there are children that need it (meaning, not children that pass through the ComponentFactory)?

Or some other pattern. Pros and cons, etc.

@caesarbell
Copy link
Collaborator Author

caesarbell commented May 29, 2024

This looks pretty good. I do think we all should probably decide on our preferred approach.

Do we use the context in DocumentBody and pass the prop through everything? Or should we use the context in each (parent) component and only pass down if there are children that need it (meaning, not children that pass through the ComponentFactory)?

Or some other pattern. Pros and cons, etc.

@mmeigs this is a good point. I think you are correct, this is probably a good spot to have that discussion.

In one case we call the context in the top level and pass it down to the ComponentFactory Component and access the boolean value for the components we need, and in those cases when there are children that do not need it, it will just have a false value by default with no objects and invalid DOM attributes rendering in the browser.

The alternative would be calling the context in all the parent components that will need it including their children, and when thinking about Typography, I feel this could be a bit overkill (CMIIW), but I am curious what you think @mmeigs and other members of the team.

I added more of the team as reviewers to get the conversation going.

@caesarbell

This comment was marked as outdated.

@caesarbell
Copy link
Collaborator Author

I have updated this PR, I believe it makes more sense to leverage the useDarkMode hook which is provided by LeafyGreen. Which also aligns with @mmeigs concerns. This allows us to select the components that need overriding styles from what LG gives us, or for Snooty Components that don't use LG.

@caesarbell caesarbell merged commit b76302b into main May 30, 2024
2 checks passed
@caesarbell caesarbell deleted the DOP-4660 branch May 30, 2024 23:49
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