-
Notifications
You must be signed in to change notification settings - Fork 172
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
Fix/dojo-bootstrap-tab-issue #168
base: master
Are you sure you want to change the base?
Conversation
Finally getting to this now... |
@apatriz if you close the panel manually we should probably unselect(unactivate) the menu item. |
good call @alaframboise |
What test app are you using? I can't seem to get any of the li's to stay active. I believe this is failing: https://github.com/Esri/calcite-maps/pull/168/files#diff-bae3f3a6daabc5281032afae882a69faR80 |
You're right, I tested again quickly last night and i ran into unexpected behavior in the 4.x sample app |
…ap tabs. add active-panel css class to differentiate bootstrap tab active status from panel active status
I think I've fixed it. Tested in all the sample apps. This is a better solution than my last attempt in my opinion -- it allows managing active status for real bootstrap tabs (like Map and Scene in the sample apps) as well as calcite-maps panels in the same dropdown menu. It will work if using 'data-toggle=tab' on panel menu items as well. |
If a panel is active on start-up, the menu UI isn't in sync. |
And we also need the jQuery equivalent. |
i'll get to the jQuery version at some point :) |
Waiting on jQuery solutions to match dojo. |
I added the jQuery implementation in my last set of commits here -- forgot to mention it! |
Finally getting to this now. Overlooked this because I'm not able to see
the inline comments you made in the code review.
If you are referring to using dojo/Nodelist-traverse -- i think i started
off without using it, but added it to make the code cleaner when querying
parent nodes. I can take a look again.
In the css, if you are referring to the .active-panel class, I added it
because of conflicts between managing calcite-panel "tabs" and bootstrap
"tabs" in the same dropdown. If data-toggle="tab" elements are used,
whether for calcite-panels or nav tabs(e.g. Map/Scene), any code that
modifies the .active class in the dropdown will create conflicts with the
dojo-bootstrap Tabs code which also acts on the .active class.
…On Tue, Oct 24, 2017 at 6:17 AM, Allan Laframboise ***@***.*** > wrote:
***@***.**** commented on this pull request.
quick review
------------------------------
In dist/css/calcite-maps-v0.4.css
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_calcite-2Dmaps_pull_168-23discussion-5Fr146555613&d=DwMCaQ&c=nE__W8dFE-shTxStwXtp0A&r=LiLFhoXOXX2WRe29xlDEiQ&m=4bKmR5bzLDJByO99t7HLnVdHKu7T8FEECL0Rtz7hEwo&s=5Tuy0nLtOPaBXuuad2PeLwvar89kQBtK3BbZRIchAdo&e=>
:
> @@ -759,7 +759,10 @@ body {
background-color: #4c4c4c; }
.calcite-dropdown.calcite-bg-dark .dropdown-menu .active > a,
.calcite-dropdown.calcite-bg-dark .dropdown-menu .active > a:hover,
- .calcite-dropdown.calcite-bg-dark .dropdown-menu .active > a:focus {
+ .calcite-dropdown.calcite-bg-dark .dropdown-menu .active > a:focus,
Is this required?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_calcite-2Dmaps_pull_168-23pullrequestreview-2D71513561&d=DwMCaQ&c=nE__W8dFE-shTxStwXtp0A&r=LiLFhoXOXX2WRe29xlDEiQ&m=4bKmR5bzLDJByO99t7HLnVdHKu7T8FEECL0Rtz7hEwo&s=hd4HrQ3Spm9EWxbg9c8ZJNvgFMRJ7wvBCaI-6M8Ao3g&e=>,
or mute the thread
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ANOvG81yUTlFhn700P1SUWVb-5F2GL9lCDks5sveODgaJpZM4OMuM0&d=DwMCaQ&c=nE__W8dFE-shTxStwXtp0A&r=LiLFhoXOXX2WRe29xlDEiQ&m=4bKmR5bzLDJByO99t7HLnVdHKu7T8FEECL0Rtz7hEwo&s=uOj2o-THBTXotCxXT0W1AbCMxtp9sf-2mTqYSYBnK7o&e=>
.
|
@apatriz finally getting back to this. I think this implementation is confusing - to have an active tab and menu item based on the active panel. |
I've been looking into an issue with dropdown menu items with the data-toggle="tab" attr staying highlighted indefinitely after clicked in a CMV application. The issue #158 seems to relate this to a dojo issue within the dijit MenuItem class.
While that solution may work, I've looked into it further and it looks like it's simply an issue with dojo-bootstrap Tab.js not selecting the dropdown-menu container by design (Tab.js line 40). You can test this by adding the data-toggle="tab" attr to any anchor element in the dropdown menu, which would normally be the way to manage tab (or panel) active state in bootstrap.
But we are using a dropdown menu and not 'tabs' in the bootstrap sense, so menu items will stay active because Tab is ignoring the parent
container. Again, this is by design based on how bootstrap tabs work normally.
So to emulate the active state management of tabs in bootstrap, it means that in addition to managing the active state of panels, we should manage the menu item active state as well.