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

Hide and hidden events on tabs #14772

Closed
wants to merge 4 commits into from
Closed

Hide and hidden events on tabs #14772

wants to merge 4 commits into from

Conversation

iamphill
Copy link

Based on #14768 I've added in two new events for tabs. hide and hidden

Follows the exact same principal as the rest of these events!

@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 10, 2014

Maybe add the new/next tab as the relatedTarget to the hide/hidden event?

@iamphill
Copy link
Author

@hnrch02 How I didn't think of that I do not know.... Will add now.

Related target contains the new tab to be shown
@iamphill
Copy link
Author

@hnrch02 Added them in & also fixed the indentation 😄

var hideEvent = $.Event('hide.bs.tab', {
relatedTarget: $this[0]
})
var showEvent = $.Event('show.bs.tab', {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, you no longer need to indent these equals signs.

Copy link
Author

Choose a reason for hiding this comment

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

@hnrch02 Do you want to me to undo them? Or leave as is? Any particular reason for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, the equals signs should line up with one another but at maximally one space after the longest variable name of the block.

var foobar = 0
var foo    = 1
var i      = 2
var bar    = 3

I hope that explains it.

Copy link
Author

Choose a reason for hiding this comment

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

@hnrch02 Make sense! I'll undo and keep them all at one space.

edit: They do line up at 1 space anyway..... 👀

</tr>
<tr>
<td>hidden.bs.tab</td>
<td>This event fires after a new tab is shown.. Use <code>event.relatedTarget</code> to target the new tab.</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

"shown.." => "shown."

@cvrebert
Copy link
Collaborator

I think the docs need to clarify the order that the 4 events are fired in.

@iamphill
Copy link
Author

Updated the docs. Did you mean something like that?

@cvrebert
Copy link
Collaborator

I had personally been thinking more like a short paragraph before the table that said "When switching to a different active tab, the events fire in the following order: ...". Just my opinion though.

An issue with the current "fires ... and before ..." phrasing is that it could be misread as the event firing twice.

@iamphill
Copy link
Author

The docs for collapse doesn't have that paragraph before it, so should that have it as well?

@hnrch02 hnrch02 added this to the v3.3.0 milestone Oct 22, 2014
@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 22, 2014

@cvrebert Can we handle the docs issue in a separate PR? I'd like to merge this one.

@cvrebert
Copy link
Collaborator

Okay

@git312
Copy link

git312 commented May 23, 2020

I´ve got some strange issues, regarding to this issue. I´m using the Bootstrap 3.4.1 Tab-Sample-HTML. After clicking a nother tab, my tabs (a-tags) are getting a display: none.

$previous.trigger(hideEvent)
$this.trigger(showEvent)

if i remove the line: $previous.trigger(hideEvent), it works. Can someone confirm this issue ? or am i the only one ? I´ve not changed anything.. it´s not working "out of the box".

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.

4 participants