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

Docs: Comparison width error #433

Closed
layershifter opened this issue Aug 24, 2016 · 6 comments
Closed

Docs: Comparison width error #433

layershifter opened this issue Aug 24, 2016 · 6 comments

Comments

@layershifter
Copy link
Member

On FHD screen I'm getting following:

image

Classes looks correctly:

image

But width taken another:

image

Looks like issue with SUI or I am missing something 💭

@levithomason
Copy link
Member

levithomason commented Aug 24, 2016

Vertical dividers are broken due to a CSS spec change. SUI core has opened issues with the w3c and Chrome: Semantic-Org/Semantic-UI#4342

Though, this might be as easy as adding largeScreen='2' to the column with the divider in it:

Introduction.js

const Comparison = ({ jsx, html }) => (
  <Segment className='code-example'>
    <Grid columns='equal' centered textAlign='left'>
      <Grid.Column mobile='16' tablet='16' computer='8' largeScreen='7'>
        <Label size='tiny' attached='top left'>JSX</Label>
        <Highlight className='language-xml'>
          {jsx}
        </Highlight>
      </Grid.Column>
-      <Grid.Column only='large screen' textAlign='center'>
+      <Grid.Column only='large screen' largeScreen={2} textAlign='center'>
        <Divider vertical>
          <Icon name='right arrow circle' />
        </Divider>
      </Grid.Column>
      <Grid.Column mobile='16' tablet='16' computer='8' largeScreen='7'>
        <Label size='tiny' attached='top right'>Rendered HTML</Label>
        <Highlight className='language-html'>
          {html}
        </Highlight>
      </Grid.Column>
    </Grid>
  </Segment>
)

The other columns are set to largeScreen='7' so there should be 2 columns left over for the middle column.

@layershifter
Copy link
Member Author

Seems, your was near, but not 😿

image

@levithomason
Copy link
Member

I am a little confused on where you are seeing this issue. Here is the current doc site in Chrome:

http://technologyadvice.github.io/stardust/introduction

doc site grid

Also, the CSS you are comparing looks correct. It should be width: 50% since that is the eight wide computer class:

image

When I make the browser full width, I also see the eight wide computer width is overridden by the seven wide large screen width:

image

Please confirm.

@layershifter
Copy link
Member Author

Chrome 52, Windows 8, 1920x1080.


image

Solution is:

- <Grid.Column mobile='16' tablet='16' computer='8' largeScreen='7'>
+ <Grid.Column mobile='16' tablet='16' computer='8' largeScreen='7' widescreen='7'>

After it:

image

@levithomason
Copy link
Member

Ah! Good call, widescreen is apparently the largest size which we were not accounting for. I wish SUI Grids were mobile first, so the larger sizes were inherited from the next largest defined size.

Either way, would you like to open a PR?

@layershifter
Copy link
Member Author

Made #438 ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants