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

Vertical Navigation: Remove shade variant #1827

Merged

Conversation

tanhengyeow
Copy link
Contributor

@tanhengyeow tanhengyeow commented Mar 9, 2019

Fixes #1573

Additional description


CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

  • First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
  • npm run lint:fix has been run and linting passes.
  • Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test).
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

  • TravisCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
  • Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
  • Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
  • Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
  • While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

@interactivellama
Copy link
Contributor

Please add a checkProp prop deprecation warning
Please remove unrelated PNGs (deleting the removed story PNG is good though).

If there's a good place to talk about how Storyshots often modifies PNGs in the contributing docs, I'd love to see something there. This a common issue.

@tanhengyeow tanhengyeow force-pushed the 1573-remove-shade-classes branch 2 times, most recently from 053ae40 to 65ed63a Compare March 14, 2019 13:44
@tanhengyeow
Copy link
Contributor Author

@interactivellama Updated the patch!

Warning attached for reference:
image

@interactivellama interactivellama added this to In progress in 2019-03 (March) via automation Apr 2, 2019
@interactivellama interactivellama self-requested a review April 2, 2019 02:22
@interactivellama interactivellama changed the title Remove shade classes from Vertical Navigation Vertical Navigation: Remove shade variant Apr 2, 2019
Copy link
Contributor

@interactivellama interactivellama left a comment

Choose a reason for hiding this comment

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

Looks good!

@interactivellama interactivellama merged commit beefa9d into salesforce:master Apr 2, 2019
2019-03 (March) automation moved this from In progress to Done Apr 2, 2019
@interactivellama
Copy link
Contributor

@tanhengyeow Thank you for contributing updated checkProps.

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

Successfully merging this pull request may close these issues.

Shade classes have been removed from Vertical Navigation
2 participants