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

Banner classes parameter does not work when type parameter is set #886

Closed
1 task done
AlexBramhill opened this issue Oct 29, 2024 · 1 comment · Fixed by #887
Closed
1 task done

Banner classes parameter does not work when type parameter is set #886

AlexBramhill opened this issue Oct 29, 2024 · 1 comment · Fixed by #887
Labels

Comments

@AlexBramhill
Copy link
Contributor

AlexBramhill commented Oct 29, 2024

Prerequisites

  • Put an X between the brackets on this line if you have done all of the following:

Description

Banner classes do not work if param.type is configured. I assume this is a bug

Steps to Reproduce

  1. Create a banner component
  2. Add a type:
{{ mojBanner({
  type: 'information',
  html: bannerHtml
}) }}
  1. Check type is rendering correctly
  2. Add a class
{{ mojBanner({
  type: 'information',
  html: bannerHtml
  classes:'aClassHere'
}) }}
  1. Note that the class is not applying correctly
  2. Remove the type
{{ mojBanner({
  html: bannerHtml
  classes:'aClassHere'
}) }}
  1. Note the class is now rendering correctly

Expected behaviour: [What you expect to happen]
Setting the classes attribute should be independent of the type attribute behaviour.

Actual behaviour: [What happens]
Setting the type attribute means classes can no longer be set

Reproduces how often: [What percentage of the time does it reproduce?]
Every time

Versions

Latest (3.0.0) -- Present for at least 2 years

Additional Information

  • Existing implementation appears to have an else and endIf block in a place that disables classes if type is set:
// Existing implementation (spacing added for readability)
<div class=
  "moj-banner
  {% if params.type == 'success' %} moj-banner--success
  {% elif params.type == 'warning' %} moj-banner--warning
  {% else %}           // This else and endIf below cause the bug
  {{- ' ' + params.classes if params.classes}}
  {% endif %}"
  ...
>
  • I've raised a PR that includes some refactoring -- The first commit is a simple fix, the latter commit provides a refactor in line with recent button-menu.

  • PR for fix here

@AlexBramhill AlexBramhill changed the title Banner - Classes parameter does not work when type parameter is set Bug: Banner - Classes parameter does not work when type parameter is set Oct 29, 2024
@AlexBramhill AlexBramhill changed the title Bug: Banner - Classes parameter does not work when type parameter is set Banner classes parameter does not work when type parameter is set Oct 29, 2024
@gregtyler
Copy link
Contributor

🎉 This issue has been resolved in version 3.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

2 participants