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

Added ChevronIcon and Expander components. #160

Merged
merged 21 commits into from
Apr 4, 2016

Conversation

nathaniellee
Copy link
Contributor

Docspot demo: http://docspot.devnxs.net/projects/lucid/feature-ANXR-190-expand-collapse/#/components/Expander

PR Checklist

  • Manually tested across supported browsers
    • Chrome
    • Firefox
    • Safari
    • IE11 (Win 7)
    • Edge (Win 10)
  • Unit tests written (common at minimum)
  • PR has one of the semver- labels
  • Two core team engineer approvals
  • One core team UX approval

@nathaniellee
Copy link
Contributor Author

I'm having some issues with the width and height of the fake button that the ChevronIcon is in specifically on Edge. I'm wondering if there's some sort of bug around how that browser handles box-sizing.

* Child element whose children represents content to be shown next to
* the expander icon.
*/
Label: any
Copy link
Contributor

Choose a reason for hiding this comment

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

We might not need this anymore since @ogupte has fixed the docs generator

Copy link
Contributor

Choose a reason for hiding this comment

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

we do still need this for the time being, i need make the docs parsing better to support lucid child components


return (
<div
{..._.omit(passThroughs, 'onExpand')}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this omit necessary? div probably ignores props it doesn't recognize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. LOL

@ogupte
Copy link
Contributor

ogupte commented Apr 2, 2016

LabeledSwitch changes should be in a different PR.

const nextLabel = _.first(Expander.Label.findInAllAsProps(nextProps)).children;

if (currentLabel !== nextLabel) {
this._labelKey++;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's sad that this is necessary :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, we couldn't find any sane way to detect if the content changed :(

nathaniellee and others added 5 commits April 4, 2016 13:03
…s/lucid into feature/ANXR-190-expand-collapse

Conflicts:
	src/components/Expander/Expander.jsx
	src/components/Expander/Expander.reducers.js
	src/components/Expander/Expander.spec.jsx
@jondlm jondlm merged commit bca486f into master Apr 4, 2016
@jondlm jondlm deleted the feature/ANXR-190-expand-collapse branch April 4, 2016 17:35
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

4 participants