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.

Sign up for GitHub

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

Jump to bottom

Promo block FE #126

Merged
merged 2 commits into from
Feb 27, 2024
Merged

Promo block FE #126

merged 2 commits into from
Feb 27, 2024

Conversation

KIRA009
Copy link
Collaborator

@KIRA009 KIRA009 commented Feb 26, 2024 •
edited
Loading

Link to Ticket

Description of Changes Made

  • Added styles to the promo block

How to Test

  • Navigate to any existing/Create a new HomePage/ServicePage page and add a promo block in the body streamfield
  • Ensure that the styles match the designs from figma (N.B. Discussed with @helenb on not using the motif for heading, because of issues with contrast)

Screenshots

Desktop image
Mobile image

MR Checklist

  • Add a description of your pull request and instructions for the reviewer to verify your work.
  • If your pull request is for a specific ticket, link to it in the description.
  • Stay on point and keep it small so the merge request can be easily reviewed.
  • Tests and linting passes.

Unit tests

  • Added
  • Not required

Documentation

Browser testing

  • I have tested in the following browsers and environments (edit the list as required)
    • Latest version of Chrome on mac
    • Latest version of Firefox on mac
    • Latest version of Safari on mac
    • Safari on last two versions of iOS
    • Chrome on last two versions of Android
  • Not required

Data protection

  • Not relevant
  • This adds new sources of PII and documents it and modifies Birdbath processors accordingly

Accessibility

  • Automated WCAG 2.1 tests pass
  • Manual WCAG 2.1 tests completed
  • I have tested in a screen reader
  • I have tested in high-contrast mode
  • Any animations removed for prefers-reduced-motion
  • Not required

Sustainability

  • Images are optimised and lazy-loading used where appropriate
  • SVGs have been optimised
  • Perfomance and transfer of data considered
  • If JavaScript is needed alternatives have been considered
  • Not required

Pattern library

  • The pattern library component for this template displays correctly, and does not break parent templates
  • The styleguide is updated if relevant
  • Changes are not relevant the pattern library

@KIRA009 KIRA009 requested a review from helenb February 26, 2024 11:42
helenb
helenb requested changes Feb 26, 2024
View reviewed changes
Copy link
Member

@helenb helenb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KIRA009 this is looking good so far, but there are a few changes needed in order to be in line with the rest of the project.

I've put some comments on the back-end ticket as the markup was mainly visible on that PR.

Please can you complete the accessibility checks and tick the relevant boxes in the MR too?

tbx/project_styleguide/templates/patterns/molecules/streamfield/blocks/promo_block.html Outdated Show resolved Hide resolved
tbx/static_src/sass/components/_promo.scss Show resolved Hide resolved
@KIRA009 KIRA009 force-pushed the feature/5445-promo-block-be branch 2 times, most recently from 24d854b to 18b5266 Compare February 27, 2024 05:39
@KIRA009 KIRA009 force-pushed the feature/0644-promo-block-fe branch 2 times, most recently from 80310cb to 3d3686d Compare February 27, 2024 06:03
helenb
helenb approved these changes Feb 27, 2024
View reviewed changes
Copy link
Member

@helenb helenb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KIRA009 looking good now.

@helenb
Copy link
Member

helenb commented Feb 27, 2024

Oh - I have approved but just noticed that this is still not done:

Please can you complete the accessibility checks and tick the relevant boxes in the MR too?

@helenb
Copy link
Member

helenb commented Feb 27, 2024

@KIRA009 there is one other thing that you could add here that would be helpful. I've just merged a change with a custom image filter which allows desaturation - we're trying it out to help reduce the image file size across the site. Could you apply it to the images used here - just pass in saturation-0.6 to the image tags.

@KIRA009 KIRA009 force-pushed the feature/5445-promo-block-be branch 2 times, most recently from 0d65a34 to 9bcf870 Compare February 27, 2024 10:26
@KIRA009 KIRA009 force-pushed the feature/0644-promo-block-fe branch from 3d3686d to e78dc2a Compare February 27, 2024 10:26
@KIRA009
Copy link
Collaborator Author

KIRA009 commented Feb 27, 2024

Updated with saturation-0.6

helenb
helenb reviewed Feb 27, 2024
View reviewed changes
tbx/project_styleguide/templates/patterns/styleguide/components/components.yaml Outdated
@@ -130,6 +130,22 @@ tags:
partner_logo max-100x90 format-webp loading="lazy" class="partners__logo":
raw: |
<img src="https://source.unsplash.com/100x90?logo" width="100" height="90" alt="" class="partners__logo">
value.image fill-520x630 as desktop_image:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing an error loading the image in the styleguide - so could you check the yaml here?

Screenshot 2024-02-27 at 10 57 41

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1d9642e

helenb
helenb reviewed Feb 27, 2024
View reviewed changes
tbx/project_styleguide/templates/patterns/molecules/streamfield/blocks/promo_block.html Outdated
{% image value.image fill-520x630 as desktop_image %}
<div class="promo-block__image promo-block__image--mobile" style="background-image: url('{{ mobile_image.url }}')"></div>
<div class="promo-block__image promo-block__image--desktop" style="background-image: url('{{ desktop_image.url }}')"></div>
{% image value.image fill-430x320 saturation-0.6 as mobile_image %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, these need to use format-webp

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added back in 1d9642e, must have gotten lost during rebase

@KIRA009 KIRA009 force-pushed the feature/0644-promo-block-fe branch from e78dc2a to 1d9642e Compare February 27, 2024 12:28
@KIRA009 KIRA009 merged commit 231c29a into feature/5445-promo-block-be Feb 27, 2024
4 checks passed
@KIRA009 KIRA009 deleted the feature/0644-promo-block-fe branch February 27, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@helenb helenb helenb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KIRA009 @helenb