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

[FEATURE] Improved Visibility styles for responsive design #429

Closed
wants to merge 7 commits into from

Conversation

3den
Copy link

@3den 3den commented Mar 18, 2017

This PR adds visibility classes that:

  • Do not mess up with the grid, by using only display: none.
  • Can be used to HIDE elements when widow is SMALLER than a given breakpoint. e.g slds-show--medium
  • Can be used to HIDE elements when widow is BIGGER (or equal) than a given breakpoint. e.g slds-hide--medium.

Issues: #427, #263, #292, #48

Example

<header>
  <nav class="slds-hide--medium">
    navbar for MOBILE, will be visible when window is smaller than medium
  </nav>
  
  <nav class="slds-show--medim slds-hide--x-large">
     navbar for DESKTOP, will be visible when window is bigger (or equal) than medium,
     but it will be hidden when it is window is x-large
  </nav>

  <nav class="slds-show--x-large">
     navbar for a VERY LARGE display, will be visible when window is bigger than x-large
  </nav>
</header>

Reviewer, please refer to this "definition of done" checklist:

  • Tested on desktop (see supported browsers)
  • Tested on mobile (for responsive or mobile-specific features)
  • Confirm Accessibility
  • Documentation is up to date
  • Release notes mention the changes

⚠️ Once this pull request is merged, please merge the code into other development branches:
Merge branch 'winter-17' into spring-17
Merge branch 'spring-17' into summer-17

@salesforce-ux-bot
Copy link
Contributor

@3den, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aputinski and @kaelig to be potential reviewers.

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @3den to sign the Salesforce Contributor License Agreement.

// [EXAMPLE] slds-show--medium:
// HIDES the element when window is SMALLER than medium.
// element will be displayed normaly when window is bigger.
.slds-show--#{$key} { display: none; }

Choose a reason for hiding this comment

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

Property 'display' should be placed on separate line from selector

// [EXAMPLE] slds-hide--medium:
// HIDES the element when window is BIGGER than medium.
// element will be displayed normaly when window is smaller.
.slds-hide--#{$key} { display: none; }

Choose a reason for hiding this comment

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

Property 'display' should be placed on separate line from selector

@rsukale
Copy link

rsukale commented Mar 20, 2017

This will make the responsiveness of slds a lot more usable and easier to understand.

@3den
Copy link
Author

3den commented Mar 21, 2017

@kaelig any update here?

@kaelig
Copy link
Contributor

kaelig commented Mar 22, 2017

I think this is a great idea, I'll let @ishakasliwal communicate it to the team!


// Generates responsive visibility tags
@each $key, $value in (
x-small: $mq-x-small,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you wrap all the keys in single quote? 'x-small'...

Copy link
Author

Choose a reason for hiding this comment

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

👍

@brandonferrua
Copy link
Contributor

This is fantastic update @3den!

Can I get you to update the documentation inside of /ui/utilties/visibility/flavors/responsive/index.react.example.jsx and /ui/utilties/visibility/flavors/responsive/index.markup.md?

Documentation will allow us to deprecate our current approach in favor of this.

Thank you again for you contribution!

@brandonferrua brandonferrua self-assigned this Mar 22, 2017
@kaelig kaelig closed this Mar 22, 2017
@kaelig kaelig reopened this Mar 22, 2017
// element will be displayed normaly when window is bigger.
.slds-show--#{$key} {
// !important is required to increase specificity
display: none!important;

Choose a reason for hiding this comment

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

! should be preceded by a space, and should not be followed by a space
!important should not be used

// element will be displayed normaly when window is smaller.
.slds-hide--#{$key} {
// !important is required to increase specificity
display: none!important;

Choose a reason for hiding this comment

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

! should be preceded by a space, and should not be followed by a space
!important should not be used

@3den
Copy link
Author

3den commented Mar 22, 2017

@brandonferrua documentation updated 💃

</tr>

Copy link
Contributor

Choose a reason for hiding this comment

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

Almost there. Since this is a markdown file, the whitespace between your tr are breaking the overview table.

image

Also, there is a paragraph block above the table that is out of date now. Could you update that as well?

Thanks!

@3den
Copy link
Author

3den commented Mar 28, 2017

@brandonferrua done :)

@3den
Copy link
Author

3den commented Mar 28, 2017

im not sure why travis build broke...

@kaelig
Copy link
Contributor

kaelig commented Mar 30, 2017

@3den I restarted it and it worked – that happens sometimes.

@ishakasliwal
Copy link
Contributor

@3den Thanks again for this contribution! We're currently updating our release process, so we'll keep you updated on the status of this merge.

@3den
Copy link
Author

3den commented Apr 4, 2017

@ishakasliwal @kaelig when is the new version of SLDS going to be released with this feature? Thanks for the support guys :)

@ishakasliwal
Copy link
Contributor

ishakasliwal commented Apr 18, 2017

This PR has been approved on our internal design systems repo and attributed to @3den . The work will be made publicly available upon our next release. Thanks for the contribution!

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

Successfully merging this pull request may close these issues.

None yet

7 participants