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

feat: add opening methods to responsiveAccordionTabs #10953 #10961

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Feb 20, 2018

Changes:

  • make ResponsiveAccordionTabs using its options
  • add open/close/toggle methods to responsiveAccordionTabs

Closes #10953

Note: open/toggle methods are not supported in tabs mode.

@DanielRuf
Copy link
Contributor

Will test tomorrow.

@DanielRuf
Copy link
Contributor

Seems fine so far. But what about historyHandled and firstTime?

@ncoden
Copy link
Contributor Author

ncoden commented Feb 21, 2018

Actually I still need to work on this. At least to improve the API for tabs. It is a bit difficult to understand what you need to pass to open between the tab and accordion markum.

So I'll take a look at historyHandled and firstTime too.

@DanielRuf
Copy link
Contributor

Do we also need some unit tests then?

@ncoden
Copy link
Contributor Author

ncoden commented Mar 19, 2018

Yes, there's never enough unit tests ;)

@DanielRuf
Copy link
Contributor

So I'll take a look at historyHandled and firstTime too.

Any news on this?

@ncoden
Copy link
Contributor Author

ncoden commented Mar 24, 2018

No, just need some time to work on this.

@DanielRuf
Copy link
Contributor

Is data-active-collapse="true" also relevant here?

@DanielRuf
Copy link
Contributor

Bump =)

@ncoden
Copy link
Contributor Author

ncoden commented Apr 28, 2018

I seen your message but as you can see this is new features planned for v6.6.0 so I'll not spend more time on this for now. There's enough work with getting v6.5.0 released. You can consider all v6.6.0 PR as freezed until v6.5.0 is out.

Thx for the bump anyway =)

@DanielRuf
Copy link
Contributor

Ah ok, was not quite clear from the changes in the comment section, only on the right side.

@ncoden ncoden added the WIP label May 14, 2018
Copy link
Contributor

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

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

Tested with patch-package in 6.4.3, works great.

@ayyobro
Copy link

ayyobro commented Nov 10, 2018

doot. Would be cool to have this merged soon!

@kball
Copy link
Contributor

kball commented Jul 26, 2019

@DanielRuf it looks like you reviewed and tested this, is it good to merge?

@DanielRuf
Copy link
Contributor

Not sure but a few things are missing but I think it can be merged as it implements most of the currently missing things.

#10961 (comment)

@kball
Copy link
Contributor

kball commented Aug 12, 2019

OK based on prior comment going to merge this

@kball kball merged commit 1f02c3e into foundation:develop Aug 12, 2019
@joeworkman
Copy link
Member

This pull request has been mentioned on Foundation Open Source Community. There might be relevant details there:

https://foundation.discourse.group/t/foundation-for-sites-v6-6-0-is-here-farout/30/1

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.

Collapsing Responsive-accordion Tabs? (Foundation 6)
5 participants