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

ModuleExpandable: added external collapsing control (expandedIndex and onExpandedChange) #1323

Merged
merged 12 commits into from Jan 20, 2021

Conversation

galaxy0101
Copy link
Contributor

@galaxy0101 galaxy0101 commented Dec 23, 2020

ModuleExpandable: added external collapsing control (expandedIndex and onExpandedChange)
with Tests and Docs examples

Kapture 2021-01-15 at 16 11 23

@galaxy0101 galaxy0101 requested a review from a team as a code owner December 23, 2020 23:57
@netlify
Copy link

netlify bot commented Dec 24, 2020

✔️ Deploy preview for gestalt ready!

🔨 Explore the source changes: 12e0c88

🔍 Inspect the deploy logs: https://app.netlify.com/sites/gestalt/deploys/60049889b420990007e816d6

😎 Browse the preview: https://deploy-preview-1323--gestalt.netlify.app

@AlbertCarreras AlbertCarreras changed the title [Gestalt] ModuleExpandable: added external control extExpandedId and setExtExpandedId for ModuleExpandable component ModuleExpandable: added external collapsing control (extExpandedId and setExtExpandedId) Jan 7, 2021
@AlbertCarreras AlbertCarreras added the minor release Minor release label Jan 7, 2021
@AlbertCarreras AlbertCarreras changed the title ModuleExpandable: added external collapsing control (extExpandedId and setExtExpandedId) ModuleExpandable: added external collapsing control (expandedId and setExpandedId) Jan 8, 2021
Copy link
Contributor

@dangerismycat dangerismycat left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! The team discussed your approach and various alternatives, and we think you're on the right track. I suggested some changes here to tighten up the API and implementation a bit. I haven't actually played around with this code, so please let me know if you run into any problems with this tweaked approach.

docs/src/Module.doc.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
docs/src/Module.doc.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
@AlbertCarreras
Copy link
Contributor

AlbertCarreras commented Jan 15, 2021

@galaxy0101 Here a few thoughts: https://www.loom.com/share/b2289b7eadab46ffab05d221b9379dc0 Code is here: #1341

@AlbertCarreras AlbertCarreras changed the title ModuleExpandable: added external collapsing control (expandedId and setExpandedId) ModuleExpandable: added external collapsing control (expandedIndex and onExpandedChange) Jan 15, 2021
Copy link
Contributor

@dangerismycat dangerismycat left a comment

Choose a reason for hiding this comment

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

This is looking great! I really like this approach now.

docs/src/Module.doc.js Outdated Show resolved Hide resolved
docs/src/Module.doc.js Outdated Show resolved Hide resolved
docs/src/Module.doc.js Outdated Show resolved Hide resolved
docs/src/Module.doc.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
packages/gestalt/src/ModuleExpandable.js Outdated Show resolved Hide resolved
@ayeshakmaz
Copy link
Contributor

Sorry for the merge conflicts @galaxy0101 - looks like we were both working on Module

Copy link
Contributor

@dangerismycat dangerismycat left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work, @galaxy0101 ! 🙂

@mergify mergify bot merged commit a157e3a into pinterest:master Jan 20, 2021
keyworks pushed a commit to keyworks/DefinitelyTyped that referenced this pull request Mar 6, 2021
keyworks pushed a commit to keyworks/DefinitelyTyped that referenced this pull request Mar 6, 2021
typescript-bot pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Mar 9, 2021
* Add eye-hide icon (pinterest/gestalt#1337)

* Add Module props and update Module.Expandable props (pinterest/gestalt#1323)

* Add zIndex prop to Typeahead (pinterest/gestalt#1350)

* Remove marginLeft and marginRight props in Box (pinterest/gestalt#1363)

* Add onNavigation props to Provider/ActivationCard/Button/DropdownItem/IconButton/Link/TapArea (pinterest/gestalt#1364)

* Add rel/target props to ActivationCard and ActionData (pinterest/gestalt#1384)

* Add ScrollBoundaryContainer types (pinterest/gestalt#1394)

* Add history icon

* Add visit icon (pinterest/gestalt#1406)

* Add type for Upsell.Form component (pinterest/gestalt#1396)

* Add Jay Kim to definition owners

* Fix lint errors

* Add tests

Co-authored-by: Jay Kim <jkim@pinterest.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor release Minor release
Projects
None yet
4 participants