Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Add persistent URLs for tab items. #413

Merged
merged 4 commits into from
Aug 5, 2015

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Aug 4, 2015

[Resolves #347]

Note: This patch adds persistent URLs to tabs on the candidate and committee detail pages (e.g. /committee/C00431445?tab=filings). It does not add URLs for more granular content, such as sub-tabs ("Compare by State", "Compare by Employer"), sort rules, map selections, etc. If we want more granular URLs, we can file additional issues.

Pinging @noahmanger for review, @onezerojeremy for thoughts on finer URL control (re.: would users like more granular controls?).

@noahmanger
Copy link
Contributor

The URLs stay persistent if you refresh, but not if you change the cycle select. The page still defaults to the first tab.

@jmcarp
Copy link
Contributor Author

jmcarp commented Aug 4, 2015

Patch updated. There are additional fixes in #411 that need to be merged for this to work completely correctly.

<ul role="tablist">
<li role="presentation" class="page-tabs__item"><a role="tab" tabindex="0" aria-controls="panel1" aria-selected="true" href="#section-1">Financial Summary</a></li>
<li role="presentation" class="page-tabs__item"><a role="tab" tabindex="-1" aria-controls="panel2" href="#section-2">Filings</a></li>
<ul role="tablist" name="tabs">
Copy link
Contributor

Choose a reason for hiding this comment

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

name shouldn't be used on <ul> and <li>s . Probably should be data-name= ?

@jmcarp
Copy link
Contributor Author

jmcarp commented Aug 5, 2015

@noahmanger thanks, fixed!

noahmanger pushed a commit that referenced this pull request Aug 5, 2015
Add persistent URLs for tab items.
@noahmanger noahmanger merged commit 6f51446 into fecgov:develop Aug 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants