-
Notifications
You must be signed in to change notification settings - Fork 29
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
refactor(website): to style markdown pages (convert info dialog page) #573
Conversation
|
||
# InfoDialog | ||
|
||
import { InfoDialog } from '@commercetools-frontend/application-components'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the interactive part. The code below is basically what we had in the VRT test.
</InfoDialog> | ||
</div> | ||
|
||
### Description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From here below is just a copy of the README
@@ -21,7 +21,7 @@ module.exports = { | |||
subgroup: [ | |||
{ | |||
label: 'InfoDialog', | |||
linkTo: '/components/dialogs/info-dialog', | |||
linkTo: '/components/info-dialog', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to keep a flat URL structure
3afb1e6
to
fff8f2e
Compare
// 'Spectral:400,700', | ||
// https://design.google/library/choosing-web-fonts-beginners-guide/ | ||
// 'Libre+Baskerville:400,400i,700', | ||
'Raleway:400,400i,700,700i', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used for the typography content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @celeste-horgan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI we do use REM in UI-Kit, so it respects the base HTML font size...
}, | ||
}, | ||
{ | ||
resolve: `gatsby-plugin-favicon`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin generates the favicons (using a webpack plugin) based on the svg logo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might actually use that for solving #257
@@ -4,23 +4,17 @@ import Highlight, { defaultProps } from 'prism-react-renderer'; | |||
import { customProperties } from '@commercetools-frontend/ui-kit'; | |||
import * as colors from '../colors'; | |||
|
|||
// TODO: improve colors | |||
const theme = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in the theme colors are based on the github
theme from prism-react-renderer
<span key={key} {...getTokenProps({ token, key })} /> | ||
))} | ||
{line.map((token, key) => { | ||
if (tokens.length === i + 1 && token.empty) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to avoid rendering an empty line at the end of the code block
`; | ||
|
||
// Typography sizes have been calculated from https://type-scale.com | ||
// Ref: https://medium.com/sketch-app-sources/exploring-responsive-type-scales-cf1da541be54 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👆Where I based the font sizes from
<svg id="Layer_1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34.3 39"><style>.st0{fill:rgb(212,4,36);} .st1{fill:rgb(242, 182, 191);} .st2{fill:rgb(16, 28, 40);} .st3{fill:rgb(159, 201, 249);}</style><g id="XMLID_1_"><path id="XMLID_75_" class="st0" d="M1.4 29.2c-.2.1-.4.4-.4.7 0 .3.2.6.4.7l14.9 7c.2.1.4.1.6.1V22.1c-.2 0-.4 0-.7.1l-14.8 7z"/><path id="XMLID_74_" class="st1" d="M17 22.1v15.6c.2 0 .5 0 .7-.1l14.9-7c.3-.1.4-.4.4-.7 0-.3-.2-.6-.4-.7l-14.9-6.9c-.3-.1-.5-.2-.7-.2z"/><path id="XMLID_73_" class="st2" d="M1 8.8c0 .3.2.6.4.7l14.9 7c.2.1.4.1.6.1.2 0 .4 0 .6-.1.1-.1.1-.1.4-.2 1.5-.7.5-.2 14.6-6.8.3-.1.5-.4.5-.7 0-.3-.2-.6-.4-.7l-15-7c-.4-.2-.9-.2-1.3 0l-14.9 7c-.2.1-.4.4-.4.7z"/><path id="XMLID_72_" class="st3" d="M17 17.5v4.6c-.2 0-.4 0-.7.1l-14.9 7c-.2.1-.4.4-.4.7V9.3v-.5c0 .3.2.6.4.7l14.9 7c.2.1.4.1.6.1.2 0 .4 0 .6-.1-.4.2-.5.4-.5 1z"/></g></svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the build errors should be fixed with #574 |
fff8f2e
to
0c01867
Compare
); | ||
const rowHeaders = tableHeaders.props.children; | ||
return React.Children.toArray(rowHeaders.props.children).reduce( | ||
(styles, elem, index) => ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
<GitHubSvg /> | ||
</a> | ||
<MenuButton onClick={props.toggleMenu}> | ||
<BurgerIcon isActive={props.isMenuOpen} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those pink underlines look weird. Shouldn't they be more inline-block than block? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks pretty nice :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, don't worry about that. I still need to go through the visual things with design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually that was easy to fix 7a08005
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return React.Children.toArray(rowHeaders.props.children).reduce( | ||
(styles, elem, index) => ` | ||
${styles} | ||
td:nth-of-type(${index + 1}):before { content: "${ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On small screens the table becomes a block list (see https://flaviocopes.com/css-responsive-table/).
This logic here is to extract the table headers and inject them into each row cell.
@@ -2,10 +2,13 @@ | |||
export const light = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The names here are still WIP, I'll clean that up when we finalize the layout
table: props => ( | ||
<div | ||
css={css` | ||
overflow: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I think from my side this PR is ready for review. The main goal is to style the markdown pages and to make the layout somehow responsive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me. I think it's important to get things started and then see where the wonky bits are and improve :)
…ame size as the text
This PR focuses on styling the markdown pages, using the InfoDialog component page as an example.
I recommend to check the deployed branch if you want to see the result (info-dialog component page)