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

Theme settings tabs are really slow to load #1174

Closed
3 tasks done
2Fwebd opened this issue Jan 3, 2016 · 50 comments
Closed
3 tasks done

Theme settings tabs are really slow to load #1174

2Fwebd opened this issue Jan 3, 2016 · 50 comments

Comments

@2Fwebd
Copy link

2Fwebd commented Jan 3, 2016

Hi there,

We have some customers complaining about the slowness of the Theme Settings pages.
As we have a lot of options and there is some PHP conditions for some of them and though a lot of JS loaded for the options. It's true that it can take a while as all is loaded in one piece.

Is there any way to deactivate that and have a different page for each tab ? So we don't load all the options but one tab at the time.

Regards,

2F


  • Tabs lazy load

    Don not render/init all tabs on page load. Render only visible tabs, then init tab on open.

    • on/off parameter for this feature in

      // file: {theme}/framework-customizations/theme/config.php
      
      $cfg['lazy_tabs'] = bool;
  • Render all tabs within some DOM elements from javascript:

    fwEvents.trigger('fw:options:init:tabs', {
        $elements: jQuery('.fw-settings-form')
    });
@ghost
Copy link

ghost commented Jan 3, 2016

Hello,

I will try to output tabs contents in <div data-tab-html="...."> then in javascript move the attribute value to actual html, doing that for each tab one by one on interval (to prevent load all at once), or if will be possible on tab focus/open. So:

  1. Initially the tab will be

    <div data-html="...html..."></div>
  2. On focus/open/activate it will become

    <div>...html...</div>

@ghost ghost added the enhancement label Jan 3, 2016
@ghost ghost self-assigned this Jan 3, 2016
@ghost ghost added the ready label Jan 3, 2016
@2Fwebd
Copy link
Author

2Fwebd commented Jan 3, 2016

Thank you @moldcraft, that'd be perfect !

@ghost ghost added in progress and removed ready labels Jan 4, 2016
ghost pushed a commit that referenced this issue Jan 4, 2016
removed the "focus tab" functionality, it's not working with this feature
@ghost
Copy link

ghost commented Jan 4, 2016

Done (on a separated branch)

But there is a problem: Some tabs are "hidden" not rendered, so a lot of inputs are not present in html, and on form submit their values are not sent in POST.

I will try to listen form submit -> prevent it -> render and init all options -> trigger submit from code. That will "block" the page (because, again, all options will be loaded at the same time) then the form will be submitted

@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

their values are not sent in POST

sorry I cant have this, I have predefined styles sent to a file and this will not send the rest of the tabs but only the visible one and the ones user has clicked on

@ghost
Copy link

ghost commented Jan 4, 2016

Done -> #1174 (comment)

Please download the latest dev version
https://github.com/ThemeFuse/Unyson/archive/2.5.0.zip
and test if the save works fine

@ghost ghost added the waiting for a reply label Jan 4, 2016
@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

cant to this man , I made bunch of layout and menu visual generator options that depend on options being there , none of them work now, you cant go change this after one year of development. I made options types depending on admin and now many of them dont work right

@ghost
Copy link

ghost commented Jan 4, 2016

So I should do an on/off parameter for this feature (enabled by default) ?

@2Fwebd
Copy link
Author

2Fwebd commented Jan 4, 2016

Hi,

I'll give it a try now. I think the best way would be to have an option in framework-customizations/theme/config.php

Like : $cfg['settings_form_side_tabs'] = true;

@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

my template generator options is not connected to save but it collects the admin options

var templatedata = $(collection + ' :input').serialize().replace(/fwf=[^\&]+\&/, '');

so if they are not rendered they cant be collected ,

I would love to see speed improvement also but I dont think that this is the right solution.

yes do on/off ( default off) but this leaves many of us that made custom options types depending on admin without an adequate speed solution.

@ghost ghost removed the waiting for a reply label Jan 4, 2016
@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

  1. what I think the biggest issue is the initialize , you render html in admin, hide it and reinitialize everything via js which kills the speed. It is not noticeable on 10-50 options ,
    but 2F has many as he said and I have 192 , that alone is a killer .
    192 seems much when said but those are a joke inputs fields that again depend on initialize. If admin were not so dependent on js it would fly.
  2. the options html is HUGE , for a simple html input we have this
    http://prntscr.com/9m1h4l

where all that can be done with 3 divs and much smaller css class names. But we have a maze of unnecessary html. These are main speed killers.

@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

here , I trimmed down all unnecessary fw-clear ( clear via css after or give those an actual css layout ) 4 per option

<div class="fw-clear"></div>

desc and help dont need inner

<div class="fw-inner"><?php echo ($option['desc'] ? $option['desc'] : '') ?></div>

<div class="fw-inner-option">

look at this
http://prntscr.com/9m1zep
http://prntscr.com/9m1zn5

that cuts 30+KB from 470KB page on 192 options and I did not even touch the tabs and JS

and here is where the mess is

http://prntscr.com/9m20hb

my admin page actually loads in 1.08sec ( used to be 3sec before this #958 )

but the JS ( show options page + initialize ) slows it down another 0.7sec

now I stayed away from all speed killers or I made my own option types to increase the speed, like slider , spinner , typography , the default ones are still killing it especially the slider

@ghost
Copy link

ghost commented Jan 4, 2016

Sorry, I designed the html structure bad at beginning.

I can do a html refactor and update all js/css selectors, if users will report/complain that their options or theme scripts used selectors dependent on .fw-backend-option-... > .fw-inner (or similar) classes, we will force them to update their selectors to match the new html structure. Sounds ok? Should I do that?

@danyj
Copy link
Contributor

danyj commented Jan 4, 2016

I dont want to break anyone's html or option that is dependent , but main hooks I used are not inner or the clears so that is why I tested the"fat trim" there.

Trimming the html , setting class names to normal size instead of this http://prntscr.com/9m4w0w and the most important one is letting HTML load without JS dependency since there is no need for it on input, text area , checkbox , radios and all normal form options types is where we should start.

Only options that need initialize are the options that their render depend on JS like slider, switch , multi-picker or typography.

think of this , from my 192 options I count 128 text inputs http://prntscr.com/9m4xpe

, which means there is no need to do js initialize 128 times on admin page. But by default we run initialize on any option type.

So yes I am for refactor but a smart and precise one , not just a new html tree but a carefully picked scan that we know it wont affect anyone.

P.S. Thank you for taking every consideration. It means that we deal with someone who cares.

@ghost ghost added this to the v2.5.0 milestone Jan 4, 2016
ghost pushed a commit that referenced this issue Jan 5, 2016
@ghost
Copy link

ghost commented Jan 5, 2016

@ghost ghost closed this as completed Jan 5, 2016
@ghost ghost removed the in progress label Jan 5, 2016
@ghost
Copy link

ghost commented Jan 5, 2016

@2Fwebd Please let us know if it fixes the slow page load problem

@2Fwebd
Copy link
Author

2Fwebd commented Jan 5, 2016

Hi there,

I just tried, it loads pretty fast on my side and the saving is working well. I'll release it to my customer and get back if that doesn't fix it but I think that'll :)

Cheers,

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

sorry for bringing this back up , can we have a callback after the tabs are loaded so that I can do this

fwEvents.trigger('fw:options:init:tabs', {
    $elements: jQuery('.fw-settings-form')
},function(){
    self.saveTemplate();
});

and this one is for theme settings only right jQuery('.fw-settings-form') ?
what do we use for tabs inside popups ?

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

Basically I need to trigger the tabs init no matter if theme settings or popup,

so shold I use this instead ?

  $elements:  jQuery('.fw-settings-form, .media-frame-content form')

@ghost
Copy link

ghost commented Mar 15, 2016

#1174 (comment)

/**
* Sometimes we want to perform some action just when
* lazy tabs are rendered. It's important in those cases
* to distinguish regular fw:options:init events from
* the ones that will render tabs. Passing by this little
* detail may break some widgets because fw:options:init
* event may be fired even when tabs are not yet rendered.
*
* That's how you can be sure that you'll run a piece
* of code just when tabs will be arround 100%.
*
* fwEvents.on('fw:options:init', function (data) {
* if (! data.lazyTabsUpdated) {
* return;
* }
*
* // Do your business
* });
*
*/
lazyTabsUpdated: true

this one is for theme settings only right jQuery('.fw-settings-form')

Yes, .fw-settings-form is for Theme Settings only

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

@moldcraft , and one more sorry , is there way to add a hook that will not continue to trigger the init if they are already initiated. Add a class to form maybe fw-tabs-initiated after the init

@ghost
Copy link

ghost commented Mar 15, 2016

Sure, nothing is initialized twice, options and containers always add an "initialized" class.

$tabs.addClass('initialized');

@ghost
Copy link

ghost commented Mar 15, 2016

Also to check if lazy tab was initialized or not, check if it has data-fw-tab-html attribute.

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

@moldcraft what I mean is this , when you click on a button I need all tabs to be initiated before render

        $(self.loadtrigger).on('click', function(e) {
            e.preventDefault();
            fwEvents.trigger('fw:options:init:tabs', {
                $elements: jQuery('.fw-settings-form,.media-frame-content form')
            });
            var templatename = $(this).attr('data-templatename');


            self.renderTemplate(templatename);

        });

now that is triggering every time and I see a lag , like it is initiating tabs every time I click on the button

@ghost
Copy link

ghost commented Mar 15, 2016

every time and I see a lag

Maybe because the css selector is slow (it's not by id or by class, it needs to check every element atts)

var selector = '.fw-options-tab[' + htmlAttrName + ']', $tabs;

Please add here console.log($tabs.length); to see how many elements are selected on button click

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

OK , this works and it eliminates additional lags, only first one lags a bit ,

            if(!$('.fw-settings-form,.media-frame-content form').hasClass('thz-tabs-init')){
                fwEvents.trigger('fw:options:init:tabs', {
                    $elements: jQuery('.fw-settings-form,.media-frame-content form')
                });
                $('.fw-settings-form,.media-frame-content form').addClass('thz-tabs-init')
            }

testing your suggestion

@danyj
Copy link
Contributor

danyj commented Mar 15, 2016

returns 4 , but without my check it returns it on every click , that is why I meant to add something to form or to tabs itself for check if they are already initiated

@ghost ghost reopened this Mar 15, 2016
@ghost ghost added ready and removed ready labels Mar 15, 2016
@ghost
Copy link

ghost commented Mar 16, 2016

I can't reproduce this in my side

@ghost ghost added ready and removed in progress labels Mar 16, 2016
@danyj
Copy link
Contributor

danyj commented Mar 16, 2016

@moldcraft you made a button that triggers this

            fwEvents.trigger('fw:options:init:tabs', {
                $elements: jQuery('.fw-settings-form,.media-frame-content form')
            });

?

@ghost
Copy link

ghost commented Mar 16, 2016

I execute the code in Console

@danyj
Copy link
Contributor

danyj commented Mar 16, 2016

ok , must be my template render since it is reloading options just like reset theme options does , so tabs are not initiated on reload. This is why a form container class hook works , I am reloading what is inside of the form.

@danyj
Copy link
Contributor

danyj commented Mar 16, 2016

either way , is MUCHHHH faster with lazy_tabs , thnx for the improvement .

@danyj
Copy link
Contributor

danyj commented Mar 16, 2016

@moldcraft , exactly like I said , http://prntscr.com/afxrmc that is after template reload ( the button click ) so re-initiation is a must on next click. No problem , it works either way.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants