Skip to content

refactor(plugins): improve how we query elements#26422

Merged
Johann-S merged 3 commits into
v4-devfrom
v4-dev-johann-collapse-less-jquery
Jun 1, 2018
Merged

refactor(plugins): improve how we query elements#26422
Johann-S merged 3 commits into
v4-devfrom
v4-dev-johann-collapse-less-jquery

Conversation

@Johann-S

@Johann-S Johann-S commented Apr 30, 2018

Copy link
Copy Markdown
Member

It should help us to be more performant on how we get our DOM elements

Should help a bit here: #26419 but we should improve collapse with multi targets

@Johann-S Johann-S force-pushed the v4-dev-johann-collapse-less-jquery branch from e1ef5ae to 220e865 Compare April 30, 2018 14:45
Comment thread js/src/collapse.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe cache the length in the loops

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it was interesting in old browsers but not anymore as far as I know

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't hurt to do it either.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

in that case I'll target that in an other PR, to fix all at once 😉

@XhmikosR XhmikosR changed the title refacto(collapse): improve how we query elements refactor(collapse): improve how we query elements Apr 30, 2018
@Johann-S Johann-S force-pushed the v4-dev-johann-collapse-less-jquery branch 2 times, most recently from 6a1e285 to 54efb31 Compare April 30, 2018 20:31
@Johann-S

Copy link
Copy Markdown
Member Author

Just a few lines please Bundlesize 😢

@Johann-S Johann-S force-pushed the v4-dev-johann-collapse-less-jquery branch from 54efb31 to 4916dfc Compare May 1, 2018 12:09
@Johann-S Johann-S changed the title refactor(collapse): improve how we query elements refactor(plugins): improve how we query elements May 1, 2018
@Johann-S Johann-S merged commit 96cbb58 into v4-dev Jun 1, 2018
@Johann-S Johann-S deleted the v4-dev-johann-collapse-less-jquery branch June 1, 2018 08:30
@mdo mdo mentioned this pull request Jun 1, 2018
@mibby

mibby commented Jul 27, 2019

Copy link
Copy Markdown

Apologies for commenting on a pull-request over a year old, but I believe this change may be causing a permanent breaking change for me with newer versions of bootstrap. Perhaps you could enlighten me if I'm missing something obvious?

I updated from Bootstrap v4.1.1 to v4.3.1 when I noticed a page stopped functioning. Testing every version in-between, the breakage seems to occur with v4.1.2. I didn't notice any outstanding breaking changes in the changelog outside of this possible change.

Relevant site script.

var all_member_id = 3187407;
function tab_setup() {
    var counter=0;
    $('.m_members').each(function() {
        counter++;
        var title = $(this).parents('.container').find(
            '.header_text_text').text();
        var tab_id = $(this).attr('class').replace(/m_members\s+/g,
            '');
        $(this).attr('data-tab', tab_id);
        if (counter == 1) {
            $('.roster_page .nav-tabs').append(
                '<li class="active"><a data-toggle="tab" href="#' +
                tab_id + '">' + title + '</a>');
            $('.roster_page .tab-content').append(
                '<div class="tab-pane active" id="' + tab_id +
                '"></div>');
            $('#' + tab_id).append($('.m_members[data-tab="' +
                tab_id + '"]'));
        } else {
            $('.roster_page .nav-tabs').append(
                '<li><a data-toggle="tab"  href="#' + tab_id +
                '">' + title + '</a>');
            $('.roster_page .tab-content').append(
                '<div class="tab-pane" id="' + tab_id +
                '"></div>');
            $('#' + tab_id).append($('.m_members[data-tab="' +
                tab_id + '"]'));
        }
    });
};

Live page functioning fine with bootstrap v4.1.1.

Test page failing to toggle data from other elements with bootstrap v4.1.2.

Full html for the specific container.
https://pastebin.com/raw/7BFVSH8q

The data elements being used for toggling are loaded individually from their own module containers.


I've also noticed in the navigation that the border-bottom line on hover cuts off in the nav-item once it reaches the carousel with Chrome. Though this is probably an entirely separate issue and perhaps a browser rendering bug.

Fine with Firefox.
link

Cut prematurely with Chrome.
link2

@Johann-S

Copy link
Copy Markdown
Member Author

Hi @mibby that's happen because you use selector which contains only numbers, and it's not a valid selector. We added information about that here: https://getbootstrap.com/docs/4.3/getting-started/javascript/#selectors

@mibby

mibby commented Jul 30, 2019

Copy link
Copy Markdown

Ah, I see. That indeed would be the problem. Thanks for the info!

Is there any way to perhaps override the selector validation? As I'm not able to change the div ids for the container modules hosting the data itself, which is where the numbers are coming from.

@Johann-S

Copy link
Copy Markdown
Member Author

@mibby You can change the href attribute by something like that: .tab-content #<your id> it'll work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants