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

Add Accordion control from https://github.com/hugoabernier/AccordionWebPart #638

Closed
wants to merge 1 commit into from

Conversation

sadomovalex
Copy link

Q A
Bug fix? [ ]
New feature? [x ]
New sample? [ ]
Related issues? fixes #X, partially #Y, mentioned in #Z

What's in this Pull Request?

Add Accordion control from https://github.com/hugoabernier/AccordionWebPart

@AJIXuMuK
Copy link
Collaborator

Hi @sadomovalex!
Thank you for the new control!

One question: what's the main reason to base it on v2 version? While we're still on v1, could it be added to v1?

@estruyf
Copy link
Member

estruyf commented Aug 24, 2020

@AJIXuMuK it is because of the dependency of Office UI Fabric 6.

@AJIXuMuK
Copy link
Collaborator

@estruyf do you see the reason for that?
I looked at the code and it seems like it could be based on v5.

@estruyf
Copy link
Member

estruyf commented Aug 24, 2020

Indeed, seems just because the usage of DefaultButton.

@estruyf
Copy link
Member

estruyf commented Aug 24, 2020

If we replace that, we can make it happen in v1.

@estruyf estruyf self-assigned this Aug 24, 2020
@estruyf
Copy link
Member

estruyf commented Aug 24, 2020

ok, seems that it was just a couple of properties that have been renamed. I made changes so that it works fine in v1. Thanks, @sadomovalex for this PR.

estruyf added a commit that referenced this pull request Aug 24, 2020
@estruyf estruyf closed this Aug 24, 2020
@sadomovalex
Copy link
Author

Great, it is even better!

@sadomovalex
Copy link
Author

@AJIXuMuK I'm not author of this control. Your thanks should go to @hugoabernier :) I just wanted to make it available with less efforts directly from npm)

@estruyf estruyf mentioned this pull request Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants