moj-frontend icon indicating copy to clipboard operation
moj-frontend copied to clipboard

fix(banner): fix issue with classes not working if banner type provided

Open AlexBramhill opened this issue 1 year ago • 0 comments
trafficstars

Identify the bug

Link to the issue describing the bug that you're fixing.

Description of the change

  • 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 %}"
  ...
>
  • Updated and refactored file to address this bug, and increase readability following pattern of declaring classes logic above component as per recent button-menu commit to main

Alternative designs

  • I have a commit on this branch (hash 5742f092b7cc670d6ab453459f03c1252964d07e) that does not do the refactor, but addresses the bug

Possible drawbacks

If this is an intended business logic, then this change is not needed.

Verification process

Manual testing:

  • [x] check can set type and component reflects this
  • [x] check can set classes and component reflects this
  • [x] check can set type and classes and component reflects this
  • [x] check if type and classes contradict, type takes precedence

Release notes

Fixed #886 where if a banner type is present, banner classes could not be set

AlexBramhill avatar Oct 29 '24 16:10 AlexBramhill