Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menu template - add additional header content container #166

Closed
guywillis opened this issue Oct 6, 2023 · 3 comments · Fixed by #169
Closed

Menu template - add additional header content container #166

guywillis opened this issue Oct 6, 2023 · 3 comments · Fixed by #169

Comments

@guywillis
Copy link
Contributor

Subject of the issue

It would be really useful to have an additional container in the menu template for the header to allow for some more unique styling.

Your environment

  • Master AT & FW

Expected behaviour

The introduction of a new container element within the .menu__header-inner element to allow for some more styling flexibility. The introduction of the element should have no visual impact to the standard view.

{{#any displayTitle subtitle body instruction}}
  <div class="menu__header boxmenu__header">
    <div class="menu__header-inner boxmenu__header-inner">

      {{#if _boxMenu._graphic._src }}
      <div class="menu__image-container boxmenu__image-container">
        <img class="menu__image boxmenu__image" src="{{_boxMenu._graphic._src}}"{{#if _boxMenu._graphic.alt}} alt="{{_boxMenu._graphic.alt}}"{{/if}}{{#unless _boxMenu._graphic.alt}} aria-hidden="true"{{/unless}}>
      </div>
      {{/if}}

      <div class="menu__header-content">

        {{#if displayTitle}}
        <div class="menu__title boxmenu__title">
          <div class="menu__title-inner boxmenu__title-inner js-heading"></div>
        </div>
        {{/if}}

        ...

      </div>

    </div>
  </div>

This header style should be achievable with the new element with very little additional CSS.

Screenshot 2023-10-06 at 11 22 21
@kirsty-hames
Copy link
Contributor

I'm in favour of having an additional container. I was undecided whether .boxmenu__image-container should sit within this but I think your suggestion above (outside of container) offers more flexibility with styling.

@guywillis
Copy link
Contributor Author

I'm in favour of having an additional container. I was undecided whether .boxmenu__image-container should sit within this but I think your suggestion above (outside of container) offers more flexibility with styling.

Thank you for taking time to review.

I had the same thought and ultimately landed on the image being outside to allow for a Text / Graphic layout in the header should it be required.

@guywillis guywillis moved this from New to Assigned in adapt_framework: The TODO Board Oct 12, 2023
@guywillis guywillis self-assigned this Oct 12, 2023
@joe-allen-89 joe-allen-89 moved this from Assigned to Needs Reviewing in adapt_framework: The TODO Board Oct 13, 2023
@github-project-automation github-project-automation bot moved this from Needs Reviewing to Recently Released in adapt_framework: The TODO Board Oct 13, 2023
@github-actions
Copy link

🎉 This issue has been resolved in version 6.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants