Fix: Add new menu header color variables (fixes #402)

[swashbuck]

Fixes #402

Fix

• Adds new color variables for the menu header: @menu-header-background-color, @menu-header-title-color, @menu-header-body-color, and @menu-header-instruction-color
• Updates schemas

[joe-allen-89]

👍

[kirsty-hames]

Although this issue is raised for Boxmenu, I would say this is a global header issue (would apply to page too). In either use case, we do have the .header-color-mixin classes in Vanilla that allow you to define a background-color for the header and a color for the text.

The differentiation between the implementation (new menu color variables vs header color mixin class) visually is the background-color is applied to the header via the class (so spans full width by default) whereas the variable applies the background-color to the header inner (so spans the content width by default).

From an AAT perspective, the color variables will be more desirable than applying classes. .header-color-mixin does also require you to pre-define colours and doesn't give you the granularity of title, body and instruction color.

I think this is a long winded way of me asking two queries,

1 - should we do the same for page header?
2 - for the application of background-color, is there a bigger use case for this to apply to the header instead of the inner? Alternatively, would we want to target the inner when a background-image is applied (as per your issue screenshot) and target the header (so span fullwidth) by default?

[swashbuck]

@kirsty-hames Thanks for the good feedback. See my thoughts below:

1. I do think that we should create color variables for the page header. I agree that specific color variables are easier to manage in the AAT than needing to remember the exact background classes to use or having to pull in a FED to create custom colors. Happy to include it as part of this PR, or we could create a new ticket for it.
2. We could just add another variable for the "outer" header. @menu-header-background-color could be used for the entire .menu__header element, and then @menu-header-inner-background-color would be used for the header inner. This would give users the most flexibility and doesn't add too much complication.

[kirsty-hames]

1. I do think that we should create color variables for the page header. I agree that specific color variables are easier to manage in the AAT than needing to remember the exact background classes to use or having to pull in a FED to create custom colors. Happy to include it as part of this PR, or we could create a new ticket for it.

Thanks @swashbuck. Whichever is easiest.

2. We could just add another variable for the "outer" header. @menu-header-background-color could be used for the entire .menu__header element, and then @menu-header-inner-background-color would be used for the header inner. This would give users the most flexibility and doesn't add too much complication.

Sounds fine to me but worth mentioning we do have a page header plugin on the horizon adaptlearning/adapt_framework#2703. I'm not sure if the intention is a replacement or alternative to the existing core menu/page headers. Either way, is this something we need to consider when reviewing this PR? I'm assuming Vanilla will need to cater for both (plugin and core) so we might need some alignment with use/naming of variables?

[swashbuck]

Sounds fine to me but worth mentioning we do have a page header plugin on the horizon adaptlearning/adapt_framework#2703. I'm not sure if the intention is a replacement or alternative to the existing core menu/page headers. Either way, is this something we need to consider when reviewing this PR?

@kirsty-hames Thanks for bringing this up. I think we could still move forward with the menu header and page header color variables and then possibly incorporate them into a new page header plugin (if and when that plugin is released). My assumption is that the current menu and page header implementations will still be supported if people don't want to use the new plugin(s). @oliverfoster Thoughts?

I'm assuming Vanilla will need to cater for both (plugin and core) so we might need some alignment with use/naming of variables?

Agree, would you want to look at this and confirm the naming conventions for the variables?

[oliverfoster]

Yea, carry on working with it as it is, we'll consider the header component later, when it's all ready.

[cahirodoherty-learningpool]

👀

[github-actions]

🎉 This PR is included in version 9.6.13 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀