Are schema _globals shared with Narrative needed in Hotgraphic schemas?

[kirsty-hames]

Subject of the issue

I started by raising an issue to update Hotgraphic as per the Narrative PR 294.

As the default mobile behaviour for Hotgraphic is Narrative, shared functionality should be updated for consistency. However, when testing this, I couldn't see that the Hotgraphic _globals previous/next are used. The Hotgraphic Narrative mobile view uses the Narrative _globals instead. Is this expected and if so, do these properties need to exist in the Hotgraphic schemas as well?

[swashbuck]

@kirsty-hames I do think we should keep them separate. When using "_isNarrativeOnMobile": false, the Narrative component is not a required dependency. So, there are use cases where courses may only have Hot Graphic but not Narrative installed.

[oliverfoster]

Hmm.. does it? Weird...

Ok, so... narrative sets up backLabel and nextLabel here https://github.com/adaptlearning/adapt-contrib-narrative/blob/c350f7610a0ee927a1259efdd2623e46e2847e94/js/NarrativeView.js#L249-L292

And hotgraphic does similar here

adapt-contrib-hotgraphic/js/hotgraphicPopupView.js

Lines 62 to 105 in 55b26b7

	setupBackNextLabels(index = this.model.getActiveItem().get('_index')) {
	const totalItems = this.model.getChildren().length;
	const canCycleThroughPagination = this.model.get('_canCycleThroughPagination');
	const isAtStart = index === 0;
	const isAtEnd = index === totalItems - 1;
	const globals = Adapt.course.get('_globals');
	const hotgraphicGlobals = globals._components._hotgraphic;
	let prevTitle = isAtStart ? '' : this.model.getItem(index - 1).get('title');
	let nextTitle = isAtEnd ? '' : this.model.getItem(index + 1).get('title');
	let backItem = isAtStart ? null : index;
	let nextItem = isAtEnd ? null : index + 2;
	if (canCycleThroughPagination) {
	if (isAtStart) {
	prevTitle = this.model.getItem(totalItems - 1).get('title');
	backItem = totalItems;
	}
	if (isAtEnd) {
	nextTitle = this.model.getItem(0).get('title');
	nextItem = 1;
	}
	}
	const backLabel = compile(hotgraphicGlobals.previous, {
	_globals: globals,
	title: prevTitle,
	itemNumber: backItem,
	totalItems
	});
	const nextLabel = compile(hotgraphicGlobals.next, {
	_globals: globals,
	title: nextTitle,
	itemNumber: nextItem,
	totalItems
	});
	this.model.set('backLabel', backLabel);
	this.model.set('nextLabel', nextLabel);
	}

Narrative does not seem to use hotgraphic labels when it's used as a replacement for hotgraphic as the labels are only ever referenced by plugin name from the _globals object.

adapt-contrib-hotgraphic/js/hotgraphicPopupView.js

Line 70 in 55b26b7

const hotgraphicGlobals = globals._components._hotgraphic;

https://github.com/adaptlearning/adapt-contrib-narrative/blob/c350f7610a0ee927a1259efdd2623e46e2847e94/js/NarrativeView.js#L257

It does make sense to keep them separate as you'd need them both installed anyway to have the switching functionality.

[kirsty-hames]

Thanks @swashbuck and @oliverfoster for looking into this. I'll close the ticket and agree to keep these separate.