Box Menu: Add variable to control item padding

[swashbuck]

Subject of the issue

Currently, the box menu item padding for .boxmenu-item__inner is hardcoded to 1rem. I would suggest that we add a new variable to control this padding value.

Perhaps these variables could be called @menu-item-inner-padding-ver and @menu-item-inner-padding-horz and would live in _spacing-containers.less.

Your environment

• Vanilla 9.16.2

Screenshots

The padding is represented by the purple highlighted area.

Example AD that requires more padding:

[guywillis]

I am all in favour of this change.

I do have some considerations though:

• Would the name of the variables @menu-item-padding-ver and @menu-item-padding-hoz be more descriptive?
  • The suggested names above are to keep consistency with other variables names at the same level @menu-item, @menu-item-inverted, '@menu-item-title' etc
  • and to keep consistency with other padding variable names @page-padding-hoz, '@block-padding-hoz' etc
• The proposed names above would mean that the current @menu-item-padding-ver and @menu-item-padding-hoz variables would need to be renamed to @menu-item-margin-ver and @menu-item-margin-hoz respectively
• The variables be placed within the _spacing-containers.less file

[swashbuck]

Would the name of the variables @menu-item-padding-ver and @menu-item-padding-hoz be more descriptive?

Yes, these names more adequately describe their purpose.

The proposed names above would mean that the current @menu-item-padding-ver and @menu-item-padding-hoz variables would need to be renamed to @menu-item-margin-ver and @menu-item-margin-hoz respectively

This makes sense as the current menu item padding variables are actually applied to margins. My only concern is breakage and backwards compatibility with custom themes. We can give the new @menu-item-margin-* variables the same defaults that they have now, but it may trip up theme developers when they find that their variable overrides are no longer working as intended.

That said, I do agree with your approach overall. Thoughts on how we could mitigate the variable renaming issues? Is this something we need to worry about? How often do you actually modify these two variables?

[guywillis]

My only concern is breakage and backwards compatibility with custom themes. We can give the new @menu-item-margin-* variables the same defaults that they have now, but it may trip up theme developers when they find that their variable overrides are no longer working as intended.

That said, I do agree with your approach overall. Thoughts on how we could mitigate the variable renaming issues? Is this something we need to worry about? How often do you actually modify these two variables?

A very valid concern.

Personally, I do not often modify these values when building. Am acutely aware that I may be the exception here though.

Perhaps simply a breaking change is required?

[kirsty-hames]

Hey @swashbuck / @guywillis, with hindsight @menu-item-margin-ver and @menu-item-margin-hoz would have been the ideal naming for the margin variables however I'm in favour of avoiding breaking changes where we can.
I think the initial suggestion of @menu-item-inner-padding-ver and @menu-item-inner-padding-horz is descriptive of where the style is applied at least, .boxmenu-item__inner and it is the inside padding. I don't mind too much that the other variables at this level don't include 'inner' in their variable name.

[github-actions]

🎉 This issue has been resolved in version 9.17.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀