Paul-DS / bootstrap-year-calendar

[DEPRECATED] A fully customizable year calendar widget, for boostrap !
Apache License 2.0
293 stars 244 forks source link

Responsive support makes switching years (rendering) very jerky #70

Open mgamulin opened 7 years ago

mgamulin commented 7 years ago

Problem is with how responsive support is implemented, month container initially has no col-xx-yy classes thus has full width and all months are rendered beneath each other.

Responsive column class is calculated with

        /* Responsive management */

        setInterval(function() {
            var calendarSize = $(_this.element).width();
            var monthSize = $(_this.element).find('.month').first().width() + 10;
            var monthContainerClass = 'month-container';

            if(monthSize * 6 < calendarSize) {
                monthContainerClass += ' col-xs-2';
            }
            else if(monthSize * 4 < calendarSize) {
                monthContainerClass += ' col-xs-3';
            }
            else if(monthSize * 3 < calendarSize) {
                monthContainerClass += ' col-xs-4';
            }
            else if(monthSize * 2 < calendarSize) {
                monthContainerClass += ' col-xs-6';
            }
            else {
                monthContainerClass += ' col-xs-12';
            }

            $(_this.element).find('.month-container').attr('class', monthContainerClass);
        }, 300);

This can be removed altogether if creation of "month-container" inside _renderBody is changed from

...
/* Container */
var monthDiv = $(document.createElement('div'));
.addClass('month-container');
...

to something like

...
/* Container */
var monthDiv = $(document.createElement('div'));
monthDiv.addClass('month-container col-xs-12 col-sm-4 col-md-3 col-lg-2');
...

It would be even better if new option for responsive column classes is introduced.

mmuman commented 7 years ago

Besides, the setInterval calls are not paired with clearInterval, so each time it adds a new timer instead of replacing it...

mmuman commented 7 years ago

I tried using those classes but in practice the months overlap quite a lot in most cases. 😕

mmuman commented 7 years ago

Oh, and it's always setting the class, instead of only setting it when it does actually change, forcing the browser to relayout everything without any valid reason every time... It's visible in the inspector, the class strings stay highlighted... Using this makes it much less CPU intensive:

                $(_this.element).find('.month-container').each(function() {
                    if ($(this).attr('class') != monthContainerClass)
                        $(this).attr('class', monthContainerClass);
                });
mmuman commented 7 years ago

I've made yet another optimization, by using a separate method that's also called at the end of render, so the 1 month/column isn't shown anymore. I'll make a PR later on.

mmuman commented 7 years ago

... and the months-container lacks the "row" class, it gets overlapped by other columns when in another grid layout...

roooodcastro commented 7 years ago

I also suggest changing this to the suggested way. There's no point using Bootstrap's responsive grid classes this way, it just hinders performance and make it difficult for others to customize things.

Also, it would be awesome if these classes (col-xs-12, col-md-4, etc) were mapped in the options object, so anyone could change them (would instantly make compatibility with Bootstrap 4 feasible)

mmuman commented 7 years ago

Oh dear, 6 months have passed. I'll try to clean this up for merge.

IvanBica commented 6 years ago

any solution for this ?

mmuman commented 6 years ago

Didn't have much time this year… I'll try to dig the patches and open a PR, but I won't have time to write a test case.

IvanBica commented 6 years ago

Fix it, you have to replace it

$(_this.element).find('.month-container').attr('class', monthContainerClass);

for

$(_this.element).find('.month-container').addClass(monthContainerClass);

mmuman commented 6 years ago

Right, addClass is cleaner.

mmuman commented 6 years ago

Hmm, looking at the code again, I don't think we want to addClass, we really want to replace the previous bootstrap class. Note monthContainerClass always contains "month-container" and is appended with the correct bootstrap layout class.

mmuman commented 6 years ago

Ok, here's my fix for it, please test. I have some more little patches (to disable the fadein, and to force the column count for things like wkhtmltopdf) but that can wait.