Add css tab to example loader #281
Conversation
DMcginn
requested review from
dkasper-was-taken,
emilyrohrbough,
mjhenkes,
ryanthemanuel and
tbiethman
as code owners
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable jsx-a11y/role-supports-aria-props */ | |||
There was a problem hiding this comment.
This disable line is to deal with an aria-haspopup is not supported by the role link linting error.
There was a problem hiding this comment.
this should be address here: #279
|
Does this handle themed examples that import themed files? Here's a link of an issue of missing vars: cerner/terra-core#3046 The changes to fix this issue would be very similar to how our components import and do theming. This would be nice to be 'face-up' if we can. |
| @@ -49,6 +50,21 @@ | |||
| display: flex; | |||
| outline: none; | |||
| padding: 0.625rem; | |||
|
|
|||
| &.is-selected { | |||
| --terra-dev-site-example-template-chevron-color: --terra-dev-site-example-template-selected-code-toggle-color; | |||
There was a problem hiding this comment.
Originally it was so that the chevrons matched the text color when the button was selected or highlighted, but I think the better option would be to just remove the color specification for the chevrons all together.
There was a problem hiding this comment.
Removed chevron coloring: f086e80.
|
|
||
| if (m !== null) { | ||
| m.forEach((match, groupIndex) => { | ||
| if (groupIndex === 1) { |
There was a problem hiding this comment.
would the first match index be 0?
There was a problem hiding this comment.
The first match is the entire import line ex: var _ActionHeaderDocCommon =_interopRequireDefault(require("./ActionHeaderDocCommon.scss", I'll look into making this shorter since there's really no reason to run through both matches.
| @@ -1,34 +1,65 @@ | |||
| const path = require('path'); | |||
| const startCase = require('lodash.startcase'); | |||
| const findCss = require('./devSiteCssFinder'); | |||
There was a problem hiding this comment.
| const findCss = require('./devSiteCssFinder'); | |
| const findCssFileName = require('./devSiteCssFinder'); |
There was a problem hiding this comment.
Updated name here: d830ef0.
| } | ||
|
|
||
| .chevron { | ||
| color: var(--terra-dev-site-example-template-chevron-color, #000); |
There was a problem hiding this comment.
I removed this originally so that the chevron could would always match the text color in the case of the tab being highlighted or selected, re-adding it and making a different text variable for it would be very simple if that's more appropriate? It would add quite a few more variables though.
| <IconChevronRight className={cx('chevron')} /> | ||
| </button> | ||
| </div> | ||
| {renderExamples()} |
There was a problem hiding this comment.
based on the naming, this should be moved to the main code snippet after render buttons
There was a problem hiding this comment.
I took renderExamples out of the renderButtons function and moved it after renderButtons: 4984d2d.
| const renderButtons = () => ( | ||
| <div> | ||
| {exampleSrc | ||
| && ( |
There was a problem hiding this comment.
we can simplify this so it's a bit easier to read
| const renderButtons = () => ( | |
| <div> | |
| {exampleSrc | |
| && ( | |
| const renderButtons = () => ( | |
| if (! exampleSrc) { | |
| return null; | |
| } | |
| return ( |
There was a problem hiding this comment.
Refactored renderButtons here: 4984d2d.
| The Example loader will take the passed in file, add it to an example template then location the source for the file in the src folder and render the example output. | ||
| The file must be a return a react component that has no required props to render. | ||
| The Example loader will take the passed in file, add it to an example template then locate the source for the file in the src folder and render the example output. | ||
| The file must be a return a react component that has no required props to render. The Example loader will also look for a css file import and render the example |
There was a problem hiding this comment.
| The file must be a return a react component that has no required props to render. The Example loader will also look for a css file import and render the example | |
| The file must be a react component that requires no additional props to render. The Example loader will also look for the first css file import and also provide the scss snippet. |
There was a problem hiding this comment.
Updated here: d830ef0.
| expect(example).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| it('returns undefined if there is no .css or .scss file declaration', () => { |
There was a problem hiding this comment.
Can we add some tests for css.moudle.scss?
Also can we add a few tests where multiple css imports are passes in to ensure the first path is resolved?
There was a problem hiding this comment.
I added a test to make sure it picked up the whole file name for css.module.scss and a second to ensure that the first import path is the one that is loaded d830ef0.
I can add more for css.module.scss if you had something different in mind?
|
I removed the jest tests for devSiteExample due to an issue with the |
Summary
Replaced the toggle background tab with a CSS tab that displays any css files imported by the example.
Closes #247
Deployment Link
https://terra-dev-si-add-css-ta-f2ejya.herokuapp.com/
Testing
There have been added WDIO tests to cover the same functionality as the existing
<code>tab tests.Additional Details
In the case that the example doesn't import a css file is the tab isn't displayed. The
<code>andcsstabs have both also been themed, the colors were based on the button-group selected and highlight colors.Thank you for contributing to Terra.
@cerner/terra