-
Notifications
You must be signed in to change notification settings - Fork 843
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
Responsive refactors and addition of helpers #909
Changes from 6 commits
5ca395e
9af775a
06a48cc
13737da
857a6df
17ec96a
a0fb1e4
2d8c321
198cee5
cf86a40
091d76e
505503d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import React from 'react'; | ||
|
||
import { | ||
EuiCode, | ||
EuiHideFrom, | ||
EuiShowFor, | ||
} from '../../../../src/components'; | ||
|
||
export default () => ( | ||
<div> | ||
<EuiHideFrom sizes={['xs']}> | ||
Hiding from <EuiCode>xs</EuiCode> screens only | ||
</EuiHideFrom> | ||
<br/> | ||
<EuiHideFrom sizes={['xs','s']}> | ||
Hiding from <EuiCode>xs, s</EuiCode> screens | ||
</EuiHideFrom> | ||
<br/> | ||
<EuiHideFrom sizes={['xs','s','m', 'l']}> | ||
Hiding from <EuiCode>xs, s, m, l</EuiCode> screens | ||
</EuiHideFrom> | ||
<br/> | ||
<EuiHideFrom sizes={['xl']}> | ||
Hiding from <EuiCode>xl</EuiCode> screens only | ||
</EuiHideFrom> | ||
|
||
<br/> | ||
<br/> | ||
|
||
<EuiShowFor sizes={['xs']}> | ||
Showing for <EuiCode>xs</EuiCode> screens only | ||
</EuiShowFor> | ||
<br/> | ||
<EuiShowFor sizes={['xs','s']}> | ||
Showing for <EuiCode>xs, s</EuiCode> screens | ||
</EuiShowFor> | ||
<br/> | ||
<EuiShowFor sizes={['xs','s','m', 'l']}> | ||
Showing for <EuiCode>xs, s, m, l</EuiCode> screens | ||
</EuiShowFor> | ||
<br/> | ||
<EuiShowFor sizes={['xl']}> | ||
Showing for <EuiCode>xl</EuiCode> screen only | ||
</EuiShowFor> | ||
</div> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import React from 'react'; | ||
|
||
import { renderToHtml } from '../../services'; | ||
import sizes from '!!sass-vars-to-js-loader!../../../../src/global_styling/variables/_responsive.scss'; | ||
|
||
import { | ||
GuideSectionTypes, | ||
} from '../../components'; | ||
|
||
import { | ||
EuiCode, | ||
EuiShowFor, | ||
EuiHideFrom, | ||
EuiCodeBlock, | ||
} from '../../../../src/components'; | ||
|
||
import Responsive from './responsive'; | ||
const responsiveSource = require('!!raw-loader!./responsive'); | ||
const responsiveHtml = renderToHtml(Responsive); | ||
|
||
function renderSizes(size, index) { | ||
let code = `'${size}': ${sizes.euiBreakpoints[size]}px`; | ||
|
||
if (index < sizes.euiBreakpointKeys.length - 1) { | ||
code += ` - ${(sizes.euiBreakpoints[sizes.euiBreakpointKeys[index+1]] - 1)}px`; | ||
} else { | ||
code += ` +`; | ||
} | ||
|
||
return ( | ||
<div key={index}> | ||
{code} | ||
</div> | ||
) | ||
} | ||
|
||
export const ResponsiveExample = { | ||
title: 'Responsive', | ||
sections: [{ | ||
title: 'EuiShowFor and EuiHideFrom', | ||
source: [{ | ||
type: GuideSectionTypes.JS, | ||
code: responsiveSource, | ||
}, { | ||
type: GuideSectionTypes.HTML, | ||
code: responsiveHtml, | ||
}], | ||
text: ( | ||
<div> | ||
<p> | ||
Pass an array of named breakpoints to either | ||
the <EuiCode>EuiShowFor</EuiCode> or <EuiCode>EuiHideFrom</EuiCode> components | ||
to make them responsive. | ||
</p> | ||
|
||
<p><strong>The sizing correlates with our <EuiCode>$euiBreakpoints</EuiCode> SASS map.</strong></p> | ||
|
||
<EuiCodeBlock language="scss" paddingSize="s"> | ||
{sizes.euiBreakpointKeys.map(function (size, index) { | ||
return renderSizes(size, index); | ||
})} | ||
</EuiCodeBlock> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would love to see this using our "blue" wrapper boxes to show people the actual sizing. It might help visualize the sizing more quickly. Even possibly listing out the more common scenarios (tablet and below is... Don't think it needs to be for this PR, and I'm happy to set it up after you merge since you did all the leg work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I can do that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @snide Here's the problem I'm running into with your suggestion. The screen and especially that side of the page never gets large enough to show the full width of anything above size 's'. Unless I'm just not understanding what you mean. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't think about this. One thing we could do is make some page outlines that were fixed positon, or just take these bars and fix them to the bottom of the page (maybe make them thinner). Always something we can do a bit later. Don't want to hold up your PR on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, ok, I'll push this idea off for later. |
||
</div> | ||
), | ||
props: { EuiShowFor, EuiHideFrom }, | ||
demo: <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.
Just a small suggestion:
EuiShowWithin
andEuiHideWithin
might reduce the cognitive overhead a bit, because I had to double-check and compare "for" vs. "from" mentally, and then I realized they actually mean the same thing. And maybe it's just me, but I usually think of a breakpoint being within a min and a max.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 hear ya with the naming, though I think "within" sounds odd. How about both of them just using "For"? This is usually how other frameworks handle it.
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.
Sounds good to me!