adaptlearning / adapt-contrib-boxMenu

A simple contributed menu based upon a grid system
GNU General Public License v3.0
2 stars 29 forks source link

New: support for responsive menu item image sources (fixes #198) #200

Closed kirsty-hames closed 2 months ago

kirsty-hames commented 3 months ago

Fixes https://github.com/adaptlearning/adapt-contrib-boxMenu/issues/198

Update

New

Testing

Please test the following use cases. All code snippets reference pre-bundled course images.

  1. Default support for src
    "_graphic": {
      "src": "course/en/images/menu-item.png",
      "alt": ""
    }
  2. Mobile and desktop images
    "_graphic": {
      "large": "course/en/images/menu-item.png",
      "small": "course/en/images/gmcq.png",
      "alt": ""
    }
  3. All device sizes
    "_graphic": {
      "xlarge": "course/en/images/vanilla-swatch.jpg",
      "large": "course/en/images/menu-item.png",
      "medium": "course/en/images/vanilla-swatch.jpg",
      "small": "course/en/images/gmcq.png",
      "alt": ""
    },

    Dependencies

    Now that .boxmenu-item__image-container is a <span> (not <div>), Vanilla margin-bottom is no longer supported (inline elements only support horizontal margins). Instead, I'd suggest applying the margin to .boxmenu-item__details as per below. See Vanilla issue https://github.com/adaptlearning/adapt-contrib-vanilla/issues/520

    .boxmenu-item {
    &__image-container + &__details {
    margin-top: @menu-item-image-margin;
    }
    }
guywillis commented 3 months ago

Not sure if it's worth mentioning but this development is only available to the Framework at present until a core contentobject.model.schema file is updated to offer up the flexibility to the AAT ( if that is something we want to do ).

kirsty-hames commented 3 months ago

Not sure if it's worth mentioning but this development is only available to the Framework at present until a core contentobject.model.schema file is updated to offer up the flexibility to the AAT ( if that is something we want to do ).

Thanks @guywillis. That's right and it's the same case for all other plugins that also use the image template. I'll update the PR message to note this.

Plugin README, example.json and schemas only include the image sources they were initially developed to support although they all support the full device range within the FW. For example, GMCQ supports small and large, whilst Accordion only supports src. If we want to extend support for AAT would we be best to do this globally for all plugin images or at least look at this holistically?

guywillis commented 3 months ago

If we want to extend support for AAT would we be best to do this globally for all plugin images or at least look at this holistically?

We'd want to implement a consistent approach globally but is outside the scope of this PR. One to bring to the weekly stand up.

kirsty-hames commented 3 months ago

If we want to extend support for AAT would we be best to do this globally for all plugin images or at least look at this holistically?

We'd want to implement a consistent approach globally but is outside the scope of this PR. One to bring to the weekly stand up.

Just FYI I've raised an issue for this https://github.com/adaptlearning/adapt-contrib-core/issues/563

github-actions[bot] commented 2 months ago

:tada: This PR is included in version 7.1.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: