Skip to content

(PDS-370) Augment existing Content styles with rules defined in Nebula #177

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

Merged
merged 10 commits into from
Nov 22, 2019

Conversation

sebastian-adam
Copy link
Contributor

@sebastian-adam sebastian-adam commented Nov 21, 2019

Resolves PDS-370

For transparency, here are the few TODOs I added to the Content component:

  • Define margins for h1 -- was not relevant for Nebula
  • Define margins for h2 -- was not relevant for Nebula
  • Create a system whereby an infinite number of ordered and unordered lists have appropriate bullets

@sebastian-adam sebastian-adam changed the title WIP: Augment existing Content styles with rules defined in Nebula Augment existing Content styles with rules defined in Nebula Nov 21, 2019
@vine77 vine77 force-pushed the tasks/import-content-styles-from-nebula branch from b508b73 to 2528c19 Compare November 21, 2019 22:13
@vine77 vine77 changed the title Augment existing Content styles with rules defined in Nebula (PDS-370) Augment existing Content styles with rules defined in Nebula Nov 21, 2019
@vine77
Copy link
Contributor

vine77 commented Nov 21, 2019

@scotje Here is the PR to upstream Nebula's in-app documentation styles to the Content component. Can you take a look to see how this would impact the Forge? I know you have also been adding styles to HTML elements inside the Content comonent (puppetlabs/puppet-forge-web:js/webpack/khepri/HowtoContent/HowtoContent.scss#L84-L113). At a glance, it looks like the img styles and code styles are different. I'd like to resolve the differences and get something that works for both Nebula and the Forge.

@sebastian-adam
Copy link
Contributor Author

sebastian-adam commented Nov 21, 2019

At a glance, it looks like the img styles and code styles are different. I'd like to resolve the differences and get something that works for both Nebula and the Forge.

Yea, I hesitated on bringing default image styles in, but decided they are helpful if the content is completely arbitrary. That said, happy to reduce or port back whatever is needed. For what it's worth, we have a a custom class in Nebula that we put on special case images. We've also done the same for a unique table cell rule.

So, per images, happy to accommodate. Per code, also happy to accommodate, but I think it's more valuable we standardize on a shared pattern there. Easier for users to jump between product documentation is my thought.

Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

Looks awesome! Just added a few comments on using our common Sass variables where we can.

Comment on lines 169 to 172
white-space: pre-wrap;
white-space: -moz-pre-wrap;
white-space: -pre-wrap;
white-space: -o-pre-wrap;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you don't need to prefix white-space: pre-wrap, which has pretty good support: https://caniuse.com/#search=white-space

Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for reviewing for the Forge Casey!

@vine77 vine77 merged commit 1a6321e into master Nov 22, 2019
@vine77 vine77 deleted the tasks/import-content-styles-from-nebula branch November 22, 2019 23:43
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