Skip to content

TWE-10 | FE | New statistics group block #334

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

Merged
merged 15 commits into from
Jan 10, 2025

Conversation

shyusu4
Copy link
Collaborator

@shyusu4 shyusu4 commented Jan 9, 2025

Link to Ticket

Description of Changes Made

BE is covered in #328

Styled the new statistics group block

  • Changed numeric colors to use the primary theme color
  • Applied the motif heading (will be updated further in TWE-28 to match the finalized design)
  • Margin adjustments for headings and intro text in stats group stylesheet

How to Test

Open the numeric_stats_group_block through the pattern library (http://localhost:8000/pattern-library/render-pattern/patterns/molecules/streamfield/blocks/numeric_stats_group_block.html)

Screenshots

Expand to see more

Screenshot (94)
Screenshot (96)

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.
  • [x ] 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

Light and dark mode

  • I have tested the changes in both light and dark mode
  • The change is not relevant to dark and light mode

Accessibility

  • Automated WCAG 2.1 tests pass
  • HTML validation passes
  • 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

@shyusu4 shyusu4 changed the title Feature/twe 10 stats block fe TWE-10 | FE | New statistics group block Jan 9, 2025
@shyusu4 shyusu4 marked this pull request as ready for review January 9, 2025 11:36
@shyusu4 shyusu4 requested a review from albinazs January 9, 2025 11:36
@shyusu4 shyusu4 changed the base branch from integration/2024-evolution to feature/twe-10-stats-block January 9, 2025 12:39
@albinazs
Copy link
Collaborator

albinazs commented Jan 9, 2025

thank you, @shyusu4!

I've noticed that the font size and weight need to be updated for the stats description and details, as well as the bottom margin of the stats block itself.

other than that, it looks good to me, and over to @chris-lawton

@albinazs albinazs requested a review from chris-lawton January 9, 2025 13:02
{% if value.title %}
<h2 class="heading heading--two">{{ value.title }}</h2>
{% include "patterns/atoms/motif-heading/motif-heading.html" with heading_level=2 heading="Lighting little fires" classes="motif-heading motif-heading--two-c motif-heading--static stats-group__heading" %}
Copy link
Member

Choose a reason for hiding this comment

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

Question: Should the heading be set from a variable or will that be implemented as part of https://torchbox.atlassian.net/browse/TWE-28?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thank you @chris-lawton - it should indeed be a variable

Copy link
Member

@chris-lawton chris-lawton left a comment

Choose a reason for hiding this comment

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

Nice work @shyusu4 - looks great!

@@ -126,4 +126,3 @@
{% block footer %}
{% include "patterns/organisms/footer/footer.html" %}
{% endblock footer %}

Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: we should always have one blank line at the end of files as it helps with git diffs - can you check your editor settings to ensure that it doesn't automatically remove them?

@@ -2,7 +2,7 @@

The site uses 'Outfit' as the main font and 'sans-serif' as the fallback font.

Outfit is avaible from [Google Fonts](https://fonts.google.com/specimen/Outfit) and is a variable font. This means that the font can be loaded as a single file and the weight and style can be adjusted using CSS. Any weight between 300 and 600 can be used.
Outfit is avaible from [Google Fonts](https://fonts.google.com/specimen/Outfit) and is a variable font. This means that the font can be loaded as a single file and the weight and style can be adjusted using CSS. Any weight between 200 and 600 can be used.
Copy link
Member

Choose a reason for hiding this comment

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

Praise: thank you for keeping the documentation up to date!

@@ -5,8 +5,8 @@
</mask>
<g mask="url(#{{ mask_id }})" transform="translate(-25.84 3.44)">
<path fill="currentColor" d="M-11-312.4h1351v696H-11z" class="quote-icon__white" />
<path d="M8.59 6.934c.1 1.181 1.73 1.554 2.406.558 3.973-5.98 9.391-11.01 13.069-12.448l-.032.66c-1.405 42.635 30.37 67.332 41.634 74.642 1.065.713 2.448-.322 1.953-1.449-6.89-15.108-3.408-28.904 9.999-41.85 2.545-2.375 5.36-5.472 7.785-8.588 0 0 2.918-3.607 5.746-8.615l.15.155c4.888 7.734 5.635 16.737 5.636 21.662.022 1.177 1.566 1.693 2.313.847 3.491-4.093 9.539-12.472 10.852-21.89 3.126-22.78-10.24-46.876-34.836-54.319-1.16-.35-2.077.781-1.418 1.769 2.038 3.333 3.152 7.944 2.872 13.81C76.072-14.556 59.433 5.29 59.433 5.29S44.76-16.077 45.407-29.643c.283-5.94 1.83-10.422 4.177-13.542a1.205 1.205 0 00-1.244-1.898C23.872-40.392 11.524-20.045 9.19-7.296c-1.052 5.609-.83 10.838-.6 14.23z" fill="var(--color--accent-two)" />
Copy link
Member

@helenb helenb Jan 10, 2025

Choose a reason for hiding this comment

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

Query: are the various changes to icons here relevant to this MR? It looks like this includes various general updates to the colours, in which case it might be worth updating the merge request description - or possibly you need a rebase in order to hide changes that are coming in from other merge requests.

Copy link
Member

Choose a reason for hiding this comment

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

I forgot to mention but I think the MR is out of date as i've seen these changes before from Albina. These are the commits I reviewed as part of this one:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @helenb, I initially picked the wrong branch to merge into for this PR and later switched it. I’ve already rebased the branch and updated it already from feature/twe-10-stats-block, so everything is up to date on my end. It looks like some changes are still showing up in the diff, probably due to a GitHub glitch and the diff didn’t update after the target branch was updated.

@helenb
Copy link
Member

helenb commented Jan 10, 2025

Thank you @shyusu4 - please excuse the drive-by review - just doing a few random pull request checks in my role as tech lead. It looks like you're doing great stuff.

@shyusu4 shyusu4 merged commit af9bb2b into feature/twe-10-stats-block Jan 10, 2025
4 checks passed
@shyusu4 shyusu4 deleted the feature/twe-10-stats-block-FE branch January 10, 2025 10:53
SharmaineLim added a commit that referenced this pull request Jan 20, 2025
* Add statistics group block to Division Page

* Add a label format to NumericStatisticsBlock

* TWE-10 | FE | New statistics group block (#334)

---------

Co-authored-by: Sherry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants