Adapt_framework: Feature request: Start controller add html class

[guywillis]

Original issue

It would be incredibly useful for Start Controller to add a custom class to the html tag. The class would be removed when the user navigates away from the start page.

Preference is for the class to be automatically defined so appropriate navigational elements, such as back or home buttons, can be removed in the base theme - e.g. .is-start-page.

It could be extended to support user defined classes if different behaviour is required across multiple start pages.

Reinterpreted as

It would be incredibly useful [...] to [be able to] add a custom class to the html tag. The class would be removed when the user navigates away from the [...] page.

Preference is for [...] [a] class to be [...] defined so appropriate navigational elements, such as back or home buttons, can be removed in the base theme [...]

It could [...] support user defined classes if different behaviour is required across multiple [...] pages.

[sarveshwar-gavhane]

PR : https://github.com/adaptlearning/adapt_framework/pull/2407

[oliverfoster]

Could we add "_htmlClassName" to the course and contentObjects instead? It would have a lot more flexibility.

https://github.com/adaptlearning/adapt_framework/blob/f1c9da9dc7d0a7d28cd9d26962e83f0934e8f320/src/core/js/router.js#L326-L330

Something like:

            var classes;
            if (Adapt.location._currentId) {
              var currentModel = Adapt.findById(Adapt.location._currentId);
              var customHTMLClasses = currentModel.get("_htmlClassName") || "";
              classes = [
                      customHTMLClasses,
                      'location-' + Adapt.location._contentType
                      'location-id-' + Adapt.location._currentId
                      ].join(" ");
            } else {
              classes = 'location-' + Adapt.location._currentLocation;
            }

But with schema updates and possibly model defaults?

[rakeshjaware-exult]

@oliverfoster Yes, definitely we can customize the name of the class. But while applying we have to check does it applicable to current page i.e. if that page is the start page if not we need to remove that class too. Also need to take all the examples into consideration as style of defining the start page is different in each example.

https://github.com/adaptlearning/adapt_framework/wiki/Content-starts-with-course.json#example-1

[oliverfoster]

The startController is responsible for selecting the correct location to start the course.
The router is responsible for ensuring the render and destruction of all locations.

The startController will still select the correct page, add "_htmlClassName": "is-start-page", and tell the router navigate to it.
The router would add/remove the "_htmlClassName" using the value from the location model.

https://github.com/adaptlearning/adapt_framework/blob/master/src/core/js/startController.js#L24
We would need to add the "is-start-page" class to the location when it is needed:

if (hasStartId) {
  var startPageClass = 'is-start-page';
  var locationModel = Adapt.findById(startId);
  if (locationModel) {
    var htmlClassName = locationModel.get('_htmlClassName') || '';
    var htmlClassNames = _.uniq(htmlClassName.split(' '));
    var hasStartPageClass = _.contains(htmlClassNames, startPageClass);
    if (!hasStartPageClass) {
      htmlClassNames.push(startPageClass);
    }
    htmlClassName = htmlClassNames.join(' ');
    locationModel.set('_htmlClassName', htmlClassName);
  }
}

Or we could add them all at the start going through each id and adding the classes? https://github.com/adaptlearning/adapt_framework/blob/master/src/core/js/startController.js#L10

We can also then use "_htmlClassName" without the start controller for other types of styling?
We could just add "is-start-page" or "hide-navigation" to the pages without the above code?

I'm not sure about the router having startController properties and logic .

[sarveshwar-gavhane]

thanks @oliverfoster for detail explanation, we can do like wise, but we already have enough classes for other type of styling without start controller.
as you said start controller will select correct page and router will add/remove the _htmlClassNameclass.

it would be good if we add _htmlClassName class directly to _start object for showing direct relation of that class with start controller as ticket suggest, but we can even add _htmlClassName class through "contentObjects".

i would like to hear from you @moloko .

[oliverfoster]

we already have enough classes for other type of styling

We don't have any predefined classes which can be applied on the page / contentObjects and which when set are applied far enough up the DOM to be able to change the navigation bar styling (they are all #wrapper or deeper except the location-id- stuff).

it would be good if we add _htmlClassName class directly to _start object for showing direct relation of that class with start controller as ticket suggest, but we can even add _htmlClassName class through "contentObjects".

That sounds fair. Although the class would only be permanently added to the page if it was visited through the startController. Such that if the user is able to return to a previously visited start page without activating the startController (direct url with no _force option, page refresh etc) then the class wouldn't have been applied. Are there any use-cases where a page would need styling for when it is visited as a start page and styling for when it is visited as a normal content page?

[oliverfoster]

Here is a common use-case that would break if the styling was added through the startController only:

1. User visits the course for their 1st attempt and lands on the start page with "_skipIfComplete": true. The startController adds styling to remove navigation buttons.
2. User watches an introduction video on the start page.
3. User completes the start page and moves on to the main menu.
4. User completes some of the content and closes the course.
5. User returns to the course - skipping the start page because it is complete - and lands on the main menu.
6. User returns to the start page from the menu, navigation buttons are now visible - this is as the startController hasn't added the classes to remove the navigation buttons.

My proposal is that, if you need to remove the navigation buttons on a page, that class should always be on that page.

[sarveshwar-gavhane]

My proposal is that, if you need to remove the navigation buttons on a page, that class should always be on that page.

Agreed.

Also we need to add _htmlClassName attribute for authoring tool support at https://github.com/adaptlearning/adapt_framework/blob/master/src/core/schema/contentobject.model.schema and
https://github.com/sarveshwar-gavhane/adapt_framework/blob/issue/2392/src/course/en/contentObjects.json

kindly confirm.
Thanks !

[oliverfoster]

you'll need it for course and contentObject as those are the main landing ids

[rakeshjaware-exult]

Understood for the contentObject but not getting why we will need it in the course

[oliverfoster]

course is a menu, just like contentObject can be. If contentObject menus can have _htmlClassName then course menus should also have it. Adapt's JSON structure is just a bit weird. Really the course object shouldn't be a piece of content.

[chucklorenz]

Having read through this issue and understanding it to mean that _htmlClassName was implemented in a way that abandoned an early association with the Start Controller, I believe that the current function of _htmlClassName is to allow the user to apply a CSS style higher in the DOM (html) than at the usual page div (child of wrapper)--hence the inclusion of "html" in the property's name.

If my understanding is correct, I'd like to share several thoughts:

• In properties.schema, _htmlClassName has "help" text, but _classes does not. The help text for _htmlClassName reassures the user that CSS will be applied to the html tag. The help for _classes keeps the user guessing. Without "help" (and user friendly titles) that distinguishes the function of the two, I believe that users will start using _htmlClassName for all classes since they have confidence about where it is being applied. We should look at improving the Title and Help text. Reference PR https://github.com/adaptlearning/adapt_framework/pull/2552
• It appears that _htmlClassName got its name when in conversation it needed to be distinguished from the Start Controller's _className. Now it is more related to _classes as it is simply a vehicle for applying CSS to the html tag. Also, the name is singular although it accommodates multiple classes. Since we are working towards a major version, perhaps this is the time to rename it to something like _htmlClasses in order to relate it to _classes. Reference PR https://github.com/adaptlearning/adapt_framework/pull/2552
• I like the opportunity to use the functionality of something like _htmlClassName. I've added similar code in the past. I'd like to retain the functionality. But I'd argue that it makes _classes on the contentObject obsolete. Everything I can do with _classes I can do with _htmlClassName--and more. But _classes (on a page) is more limiting than _htmlClassName because _classes is a subset of the DOM. Whereas _classes on articles and blocks makes sense because those elements can appear multiple times, no one is proposing having multiple pages appearing within a single wrapper. If we keep both _htmlClassName and _classes on the page, we should anticipate having to explain when/why it is better to use one than the other. For the AAT user, the issue is further muddied by the presence of "Custom CSS/Less code" on the same configuration page (Project Settings)--how to distinguish between the three options.

In case this resounds with you, I created Reference PR https://github.com/adaptlearning/adapt_framework/pull/2552. It addresses the first 2 points but not the third. (I don't have a recommendation to resolve this.)

[guywillis]

Thank you for the detailed breakdown, Chuck. I agree with both of your first two points.

With regards to point three, I would argue that _classes on the contentObject is still valuable as it allows you to add a custom class to the associated item on the menu. Adapt will also attach anything put in _classes to the .page element but I agree _htmlClassName is more useful for styling.

[moloko]

can this be closed now?

[chucklorenz]

As far as it concerns me, yes, it can be closed.