NUKnightLab / TimelineJS3

TimelineJS v3: A Storytelling Timeline built in JavaScript. http://timeline.knightlab.com
Mozilla Public License 2.0
3k stars 621 forks source link

Can i change the flag colours to different different colours for different group items #489

Open jiwitesh opened 7 years ago

jiwitesh commented 7 years ago

Can i change the flag colours to different different colours for different group items.So that it becomes easy for the readers to understand which news is related to which group.Is it possible

mastef commented 7 years ago

I changed the setActive function to this :

setActive: function(is_active) {
    this.active = is_active;

    if (this.active && this.has_end_date) {
        this._el.container.className = 'tl-timemarker tl-timemarker-with-end tl-timemarker-active';
    } else if (this.active){
        this._el.container.className = 'tl-timemarker tl-timemarker-active';
    } else if (this.has_end_date){
        this._el.container.className = 'tl-timemarker tl-timemarker-with-end';
    } else {
        this._el.container.className = 'tl-timemarker';
    }

    if (!!this.data.group) {
        this._el.container.className = this._el.container.className + ' has-group-marker ' + TL.Util.slugify(this.data.group) + '-group-marker';
    }else{
        this._el.container.className = this._el.container.className + ' no-group-marker';
    }
},

This way timeline items with groups have a new css class assigned : has-group-marker and <groupname>-group-marker. If there's no group class, then it will have a css class no-group-marker assigned.

Styling for e.g. a group called location then works like this :

.location-group-marker.tl-timemarker .tl-timemarker-content-container {
    background-color: purple;
}
JoeGermuska commented 7 years ago

I'm not confident that the value of this.data.group is a valid CSS class name (Stack Overflow summary of rules).

That doesn't mean this solution couldn't be refined to get there, although there's also the challenge of expressing the transformation rules somewhere where people can read and understand them.

mastef commented 7 years ago

Fair enough, adjusted code to include TL.Util.slugify(this.data.group). A group called work & travel now ends up with the class work-travel-group-marker.

( TL.Util.slugify is already used in the creation of the unique element IDs for each slide. )

bjhewitt commented 7 years ago

Is there a branch associated with that suggested change/addition?

bjhewitt commented 7 years ago

I was thinking of removing the extraneous stuff from setActive, so it's not hard-setting classes it shouldn't be dealing with:

setActive: function(is_active) {
        this.active = is_active;

        if (this.active && !(this._el.container.classList.contains('tl-timemarker-active')) ) {
            this._el.container.classList.add('tl-timemarker-active');
        }

        else if (this._el.container.classList.contains('tl-timemarker-active')) {
            this._el.container.classList.remove('tl-timemarker-active');
        }
    },

then add a "_setMarkerGroup" to the markers methods:

_setMarkerGroup: function() {
        for (var i = 0; i < this._markers.length; i++) {
            if (this._markers[i].data.group.length > 0 ) {
                var groupClass = 'tl-timemarker-group--' + TL.Util.slugify(this._markers[i].data.group);
                this._markers[i]._el.container.classList.add(groupClass);
            }
        }
    },

and call it during _drawTimline:

_drawTimeline: function(fast) {
        this.timescale = this._getTimeScale();
        this.timeaxis.drawTicks(this.timescale, this.options.optimal_tick_width);
        this._positionMarkers(fast);
        this._setMarkerGroup();
        this._assignRowsToMarkers();
        this._createGroups();
        this._positionGroups();

        if (this.has_eras) {

            this._positionEras(fast);
        }
    },

Is that sensible?

mastef commented 7 years ago
  1. It would break some backwards compatibility since it doesn't handle the this.has_end_date condition it replaces.
  2. Instead of classList you can use TL.DomUtil ( see https://github.com/NUKnightLab/TimelineJS3/issues/223 and https://github.com/NUKnightLab/TimelineJS3/commit/ffe61d12bd3c3b8037aec666d612278873ece1cc )
  3. As for the function name I'd suggest _groupMarkers() so it fits with the other Marker function names. Not sure if you then would need to expose it in public like _positionMarkers is exposed.
bjhewitt commented 7 years ago

Does setActiveState need to handle this.has_end_date? It doesn't change, and it's handled in _initLayout:

        if (this.data.end_date) {
            this.has_end_date = true;
            this._el.container.className = 'tl-timemarker tl-timemarker-with-end';
        }

Or is there some other reason to do it in setActiveState?

mastef commented 7 years ago

I'm not sure, it's out of the scope of my problem tbh.

Seems like there was a need for that when it was made the first time. Maybe because the value could change on resizing, after render or for animations?

I propose to make a new issue if this refactor is needed and check in the commit history when it was added and why.