fix: return to initial state when reseting hash in Accordion & Tabs #11100 #11477

ncoden · 2018-08-26T21:45:31Z

Description

When deep-linking is enabled and the user goes back in the history up to when no hash is set, the active panel or tab does not change and is still the one that was selected the first.

This pull request make Accordion and Tabs displaying the panel or tab that was active at the component initialization.

Changes:

Save up the active panel/tab at Accordion/Tabs initialization (_initialAnchor)
Display the initial panel/tab when the hash change to "".

Types of changes

Documentation
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (anything that would change an existing functionality)
Maintenance (refactor, code cleaning, development tools...)

Checklist

I have read and follow the CONTRIBUTING.md document.
The pull request title and template are correctly filled.
The pull request targets the right branch (develop or develop-v...).
My commits are correctly titled and contain all relevant information.
~~I have updated the documentation accordingly to my changes (if relevant).~~
~~I have added tests to cover my changes (if relevant).~~

…oundation#11100 When deep-linking is enabled and the user goes back in the history up to when no hash is set, display the initially-selected content/tab instead of staying with the content/tab that was selected the first. Closes foundation#11100 Replaces foundation#11101 Co-authored-by: Jamie Chong <jamie@hippocurious.com>

SassNinja

@ncoden I've tested the accordion and it works – but only if an item is initially active!

If no item has initially the class is-active is fails.
I think we should cover that case, too.

SassNinja · 2018-08-29T10:10:22Z

js/foundation.tabs.js

      //need a hash and a relevant anchor in this tabset
-      if(anchor.length) {
+      if (anchor.length) {
        var anchorNoHash = (anchor.indexOf('#') >= 0 ? anchor.slice(1) : anchor);
        var $link = this.$element.find(`[href$="${anchor}"],[data-tabs-target="${anchorNoHash}"]`).first();
        if ($link.length) {


In the course of testing the tabs I noticed the focus style remains when using history back.

What do you think about auto focusing the link to fix this?

if ($link.length) { $link.focus(); this.selectTab($(anchor), true);

This is an other issue, not related to this pull request. Why not, but I wonder what's the impact on accessibility to move the focus like this.

Talking abt accessibility: I've noticed the tabindex gets always set to -1 for the inactive tabs what makes it impossible to navigate through the tabs via keyboard (quite bad imo)

but you're right, it's not supposed to be part of this PR

When going back in history, the Accordion without any panel active is still considered as in its "initial state".

Split DOM operations from logics in Accordion "up" and "down" methods

ncoden · 2018-08-30T22:10:03Z

@SassNinja I refactored the component to clean up a bit the logic and split the options processing/component logic from simple DOM manipulations. It should resolve the first bug you pointed out.

ncoden · 2018-08-31T21:24:08Z

@SassNinja Could you review this again please ? :)

SassNinja · 2018-09-08T21:30:53Z

Could you review this again please ? :)

It's working better now: history back adjust the adressbar as expected.

But the active state is not working as expected:
when I start without an initially opened tab, click on a tab and go back in history, I don't get the original active state (none tab opened). Instead the previously clicked tab is active (although the page url has changed).

ncoden · 2018-09-08T21:34:29Z

Shall I provide an example repo or is it clear what I mean?

~~I understood the issue, and thought I resolved it. If you look at the code, you can see that _closeAll() is called if we go back to a state where no link is active.~~

ncoden · 2018-09-08T21:43:16Z

Actually, I fixed this issue in the Accordion component, but not in Tabs. 😄

SassNinja · 2018-09-08T22:54:30Z

Just to be clear: the accordion component works as expected.
The issues described above only refers to the tabs components.

Here's a quick gist with the content of my \test\visual\tabs\deep-linking.html
https://gist.github.com/SassNinja/c194da0830de81a510061c99ffcb8938

ncoden · 2018-09-08T22:58:00Z

Just to be clear: the accordion component works as expected.
The issues described above only refers to the tabs components.

Yeah I got it. I need to do the same refactor in Tabs as I did in Accordion to resolve it. I'll take care of this sunday.

See foundation#11477 (comment)

ncoden · 2018-09-10T20:38:55Z

@SassNinja I made the requested changes. 👋 Could you test it again?

SassNinja · 2018-09-12T14:19:49Z

@ncoden lgtm 👍

Just two things don't look nice

focus state doesn't get reseted on history back (as mentioned earlier)
the hash tag in the browser url doesn't get removed when I toggle a tab, so that no panel is opened (when using data-active-collapse="true")

But nothing related to this PR – so good to merge imo.

…nking-11100 for v6.5.0 5ea51f3 fix: return to initial state when reseting hash in Accordion & Tabs foundation#11100 314b4ee fix: fix initial state for Accordion without any panel active abbbdbf refactor: split DOM operations from logics in Accordion e261905 refactor: move "init only" Accordion logic to its own methods b819faf docs: remove outdated comments in Accordion related to the initial state 7307c43 feat: add "Tabs._collapse()" to collapse the active tab 012db10 refactor: clean up the Tab initialization c6d37c4 fix: close the active Tab when going back to a "tab-less" history state 2673567 docs: fix "zplugin" typo in Accordion & Tabs events Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>

ncoden added Accordion Tabs bug fix labels Aug 26, 2018

ncoden requested review from DanielRuf and designerno1 August 26, 2018 21:45

ncoden mentioned this pull request Aug 26, 2018

feat: add class for active tab when no hash is set in Tabs #11101

Closed

ncoden force-pushed the fix/initial-tab-deep-linking-11100 branch from d01d8b6 to 5ea51f3 Compare August 26, 2018 21:48

SassNinja self-requested a review August 29, 2018 09:48

SassNinja suggested changes Aug 29, 2018

View reviewed changes

ncoden added 4 commits August 29, 2018 23:57

fix: fix initial state for Accordion without any panel active

314b4ee

When going back in history, the Accordion without any panel active is still considered as in its "initial state".

refactor: split DOM operations from logics in Accordion

abbbdbf

Split DOM operations from logics in Accordion "up" and "down" methods

refactor: move "init only" Accordion logic to its own methods

e261905

docs: remove outdated comments in Accordion related to the initial state

b819faf

ncoden added 4 commits September 9, 2018 18:30

feat: add "Tabs._collapse()" to collapse the active tab

7307c43

refactor: clean up the Tab initialization

012db10

fix: close the active Tab when going back to a "tab-less" history state

c6d37c4

See foundation#11477 (comment)

docs: fix "zplugin" typo in Accordion & Tabs events

2673567

ncoden added this to the 6.5.0 milestone Sep 10, 2018

SassNinja approved these changes Sep 12, 2018

View reviewed changes

ncoden merged commit 0358198 into foundation:develop Sep 12, 2018

ncoden mentioned this pull request Sep 12, 2018

🚀 Release v6.5.0-rc.3 #11499

Merged

10 tasks

jnns mentioned this pull request Oct 8, 2018

Regression: Accordion scrolls to first anchor even if no deeplink was supplied #11527

Closed

3 tasks

ncoden mentioned this pull request Oct 9, 2018

fix: fix various bugs in Tabs & Accordion deep linking #11527 #11530

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: return to initial state when reseting hash in Accordion & Tabs #11100 #11477

fix: return to initial state when reseting hash in Accordion & Tabs #11100 #11477

ncoden commented Aug 26, 2018 •

edited

Loading

SassNinja left a comment

SassNinja Aug 29, 2018

ncoden Aug 29, 2018

SassNinja Aug 29, 2018

ncoden commented Aug 30, 2018

ncoden commented Aug 31, 2018 •

edited

Loading

SassNinja commented Sep 8, 2018 •

edited

Loading

ncoden commented Sep 8, 2018 •

edited

Loading

ncoden commented Sep 8, 2018 •

edited

Loading

SassNinja commented Sep 8, 2018

ncoden commented Sep 8, 2018

ncoden commented Sep 10, 2018 •

edited

Loading

SassNinja commented Sep 12, 2018

fix: return to initial state when reseting hash in Accordion & Tabs #11100 #11477

fix: return to initial state when reseting hash in Accordion & Tabs #11100 #11477

Conversation

ncoden commented Aug 26, 2018 • edited Loading

Description

Changes:

Types of changes

Checklist

SassNinja left a comment

Choose a reason for hiding this comment

SassNinja Aug 29, 2018

Choose a reason for hiding this comment

ncoden Aug 29, 2018

Choose a reason for hiding this comment

SassNinja Aug 29, 2018

Choose a reason for hiding this comment

ncoden commented Aug 30, 2018

ncoden commented Aug 31, 2018 • edited Loading

SassNinja commented Sep 8, 2018 • edited Loading

ncoden commented Sep 8, 2018 • edited Loading

ncoden commented Sep 8, 2018 • edited Loading

SassNinja commented Sep 8, 2018

ncoden commented Sep 8, 2018

ncoden commented Sep 10, 2018 • edited Loading

SassNinja commented Sep 12, 2018

ncoden commented Aug 26, 2018 •

edited

Loading

ncoden commented Aug 31, 2018 •

edited

Loading

SassNinja commented Sep 8, 2018 •

edited

Loading

ncoden commented Sep 8, 2018 •

edited

Loading

ncoden commented Sep 8, 2018 •

edited

Loading

ncoden commented Sep 10, 2018 •

edited

Loading