UHF-9132: Add max-width limitation to all content types without sidebar except landing page #811

teroelonen · 2023-10-26T09:23:05Z

UHF-9132

What was done

In short the purpose of this PR is to limit the component--upper region to max-width of 860px like some of the elements that were already on the page.
I can't say this is very elegant and it still has some lists for the paragraphs that should be full width so it is not a perfect solution. If you can think of a better solution for the changes I am more than happy to listen.

How to install

Make sure your instance is up and running on latest dev branch.
- git pull origin dev
- make fresh
Update the HDBT theme
- composer require drupal/hdbt:dev-UHF-9132
Run make drush-cr

How to test

This PR should be checked with at least SOTE, KYMP, REKRY, Front page and HALLINTO instances.
Basically you need to check each content type with each design alternative of each paragraph that you are able to add on the page. This would obviously be too much of a task for anyone at this point so my recommendation is to do some checks by browsing the sites freely and try to see if there is some visual issues somewhere.
Check that the paragraphs that have gray/black bg for example go from edge to edge on mobile and work nicely on desktop.
Check that basic page, landing page, TPR unit and TPR service, news page, job application, district and project pages look functional. The district and project pages are KYMP specific pages and to test them you need to also checkout the KYMP specific PR linked on this PR. Also check the KASKO and REKRY specific PRs for their changes.
Check that code follows our standards.

Designers review

This PR does not need designers review
This PR has been visually reviewed by a designer (Sami)

Other PRs

…ar except landing page, fix padding issues for event-list, map, service-list and unit-search paragraphs

…drupal-hdbt into UHF-9132

…ent cards with gray bg

…drupal-hdbt into UHF-9132

…bar spacing

…drupal-hdbt into UHF-9132

…ainer

…lock

…nent

…omponent.scss

…nki/drupal-hdbt into UHF-9132

… class

…idth class

…use component--full-width class

…raphs

…search and event-list paragraphs

…nki/drupal-hdbt into UHF-9132

…m pages

…/City-of-Helsinki/drupal-hdbt into UHF-9132

…drupal-hdbt into UHF-9132

…nki/drupal-hdbt into UHF-9132

khalima

The functionality works. I tested these in Kasko, Sote, Kymp, Strategia and Etusivu. Found some bugs, but all should be fixed now as we checked all issues together with @teroelonen.

@Arkkimaagi Can you check these changes and approve this PR once you think all is good.

…nki/drupal-hdbt into UHF-9132

…component:not(.component--full-width)

Arkkimaagi

Wow! Just WOW!

What a great job you have done! So much removed lines, cleared code and nice work!

I found some minor nitpicks (as usual), but I could not find anything major. What an exellent job!

Arkkimaagi · 2023-11-30T09:48:39Z

templates/paragraphs/paragraph--service-list-search.html.twig

@@ -3,7 +3,7 @@
 {% block paragraph %}
  {% embed "@hdbt/misc/component.twig" with
    {
-      component_classes: [ 'component--service-list-search' ],
+      component_classes: [ 'component--full-width', 'component--service-list-search' ],


I'd recommend putting these on separate rows for readability and better git workflows.

Changed this 👍

Arkkimaagi · 2023-11-30T09:48:56Z

templates/paragraphs/paragraph--unit-search.html.twig

@@ -4,7 +4,7 @@
 {% block paragraph %}
  {% embed "@hdbt/misc/component.twig" with
    {
-      component_classes: [ 'component--unit-search' ],
+      component_classes: [  'component--full-width', 'component--unit-search' ],


I'd recommend putting these on separate rows for readability and better git workflows.

Changed this 👍

Arkkimaagi · 2023-11-30T09:50:38Z

templates/component/map.twig

@@ -1,6 +1,6 @@
 {% embed "@hdbt/misc/component.twig" with
  {
-    component_classes: [ 'component--map' ],
+    component_classes: [ 'component--map', 'component--full-width' ],


I'd recommend putting these on separate rows for readability and better git workflows.

Changed this 👍

src/scss/06_components/paragraphs/_content-cards.scss

src/scss/06_components/layout/_component.scss

Arkkimaagi · 2023-11-30T09:59:12Z

src/scss/06_components/layout/_component.scss

+  }
+}
+
+// Unit needs also these following fixed elements max-width. This should/could be fixed


Should we have a real TODO here with ticket id to fix it at some time?

Added a TODO here and a ticket to Jira :)

Arkkimaagi · 2023-11-30T09:59:18Z

src/scss/06_components/layout/_component.scss

+  max-width: $content-width-max
+}
+
+// Service needs also these following fixed elements max-width.  This should/could be fixed


Should we have a real TODO here with ticket id to fix it at some time?

Added a TODO here and a ticket to Jira :)

Arkkimaagi · 2023-11-30T10:01:05Z

src/scss/06_components/layout/_layout.scss

@@ -1,4 +1,4 @@
-$-sidebar-width: 340px;
+$-sidebar-width: calc(340px + $spacing-double + $spacing-double);


Maybe explain from where these $spacing-double values come from so that the math can be adjusted if needed at some point.

Added comment above to clarify this calc.

src/scss/06_components/layout/_layout.scss

Arkkimaagi · 2023-11-30T10:07:11Z

src/scss/06_components/paragraphs/_content-cards.scss

+  padding-top: 0;
+
+  @include breakpoint($breakpoint-m) {
+    grid-template-columns: [full-width-start] $spacing-double [content-start] 1fr [content-end] $spacing-double [full-width-end] ;


I'd consider using a custom property (css variable) for the gap, so that only it changes and we do not need to define the full grid-template-columns twice. Maybe set it without the mediaquery as

grid-template-columns: [full-width-start] var(--component-gap, #{$spacing}) [content-start] 1fr [content-end] var(--component-gap, #{$spacing}) [full-width-end];

and then in the mediaquery just set --component-gap: #{$spacing-double};. This way it's easier to see what we're doing at the breakpoint and to adjust the template-columns if needed.

This was awesome! When writing this I didn't like the fact that I had to define the template columns twice but didn't think of a better solution. Thank you :) You made me a better developer again 🙇

… are not needed, adjusting the content cards gray styles to make it more maintainable, adding two TODOs

…drupal-hdbt into UHF-9132

…n content cards gray

…drupal-hdbt into UHF-9132

Arkkimaagi

Good job! Approved 🌹

UHF-9132: Add max-width limitation to all content types without sideb…

09a5725

…ar except landing page, fix padding issues for event-list, map, service-list and unit-search paragraphs

teroelonen added 23 commits November 20, 2023 12:27

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

41f481b

…drupal-hdbt into UHF-9132

UHF-9132: Fix issues on components upper region with columns and cont…

2d33479

…ent cards with gray bg

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

285a225

…drupal-hdbt into UHF-9132

UHF-9132: Removed has-sidebar spesific styles and adjusted first-side…

6b9cef7

…bar spacing

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

0867caa

…drupal-hdbt into UHF-9132

UHF-9132: Add class to indicate components--upper region on tpr units

407cccc

UHF-9132: Move hard-coded components under the components--upper cont…

2f178c7

…ainer

UHF-9132: Move additional information under the contact information b…

d304b45

…lock

UHF-9132: Sidebar component spacing fix

7ac2ce4

UHF-9132: Add component--full-width class and assaign it to map compo…

2ae1891

…nent

UHF-9132: UHF-9132: Add project-list component to exception list on c…

4a23e94

…omponent.scss

UHF-9132: Merge branch 'UHF-9132' of https://github.com/City-of-Helsi…

18ebd19

…nki/drupal-hdbt into UHF-9132

UHF-9132: Convert remote video paragraph to use component--full-width…

4e8bfd3

… class

UHF-9132: Convert content cards gray version to use component--full-w…

063e1f2

…idth class

UHF-9132: Convert event list, service list search and unit search to …

292cee1

…use component--full-width class

UHF-9132: Add list of plans paragraph to the list of full-width-parag…

46d046a

…raphs

UHF-9132: Add component--full-width classes for district and project …

24b5b87

…search and event-list paragraphs

UHF-9132: Layout adjustments to fix issues on for example news archive

c22634e

UHF-9132: Merge branch 'UHF-9132' of https://github.com/City-of-Helsi…

5f9d254

…nki/drupal-hdbt into UHF-9132

UHF-9132: Small layout fixes for news archive

0a2573b

UHF-9132: Merge branch 'UHF-9132' of https://github.com/City-of-Helsi…

fa22f2a

…nki/drupal-hdbt into UHF-9132

UHF-9132: Fix issues with layout changes in news archive and news ite…

27332f3

…m pages

UHF-9132: Fix issue with cookie compliance page padding

c39d39f

teroelonen mentioned this pull request Nov 23, 2023

UHF-9132: Add max-width limitation to all content types without sidebar except landing page City-of-Helsinki/drupal-helfi-kymp#734

Merged

4 tasks

teroelonen marked this pull request as ready for review November 23, 2023 15:20

teroelonen added 3 commits November 24, 2023 10:19

UHF-9132: Merge branch 'UHF-9354_node_packages' of https://github.com…

8d22416

…/City-of-Helsinki/drupal-hdbt into UHF-9132

UHF-9132: Merge branch 'UHF-9354_node_packages' of https://github.com…

693e940

…/City-of-Helsinki/drupal-hdbt into UHF-9132

UHF-9132: Merge branch 'UHF-9354_node_packages' of https://github.com…

7c29e98

…/City-of-Helsinki/drupal-hdbt into UHF-9132

teroelonen added 4 commits November 28, 2023 10:02

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

bd4b2c1

…drupal-hdbt into UHF-9132

UHF-9132: Merge branch 'UHF-9132' of https://github.com/City-of-Helsi…

667357f

…nki/drupal-hdbt into UHF-9132

UHF-9132: Fix issues with news archive & news_listing__item

a6d8c18

UHF-9132: Add more specific class for content tags selector

5741588

khalima reviewed Nov 28, 2023

View reviewed changes

teroelonen added 3 commits November 29, 2023 16:21

UHF-9132: Remove selectors that are not required for the styles to apply

4cc8c52

UHF-9132: Merge branch 'UHF-9132' of https://github.com/City-of-Helsi…

bbc9f4e

…nki/drupal-hdbt into UHF-9132

UHF-9132: Replace dumb list of selectors with single selector on the …

b2a7fda

…component:not(.component--full-width)

Arkkimaagi suggested changes Nov 30, 2023

View reviewed changes

teroelonen added 5 commits December 1, 2023 11:18

UHF-9132: Small readability adjustments, removing some selectors that…

7ba5895

… are not needed, adjusting the content cards gray styles to make it more maintainable, adding two TODOs

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

50bf95f

…drupal-hdbt into UHF-9132

UHF-9132: Update comment

64250a4

UHF-9132: Fix missing initial value from the component-gap variable i…

58f12a5

…n content cards gray

UHF-9132: Merge branch 'main' of https://github.com/City-of-Helsinki/…

24776d0

…drupal-hdbt into UHF-9132

Arkkimaagi approved these changes Dec 1, 2023

View reviewed changes

teroelonen merged commit 23f88b1 into main Dec 1, 2023
1 check passed

teroelonen deleted the UHF-9132 branch December 1, 2023 12:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UHF-9132: Add max-width limitation to all content types without sidebar except landing page #811

UHF-9132: Add max-width limitation to all content types without sidebar except landing page #811

teroelonen commented Oct 26, 2023 •

edited

Loading

khalima left a comment

Arkkimaagi left a comment

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi Nov 30, 2023

teroelonen Dec 1, 2023

Arkkimaagi left a comment

		@@ -1,4 +1,4 @@
		$-sidebar-width: 340px;
		$-sidebar-width: calc(340px + $spacing-double + $spacing-double);

UHF-9132: Add max-width limitation to all content types without sidebar except landing page #811

UHF-9132: Add max-width limitation to all content types without sidebar except landing page #811

Conversation

teroelonen commented Oct 26, 2023 • edited Loading

What was done

How to install

How to test

Designers review

Other PRs

khalima left a comment

Choose a reason for hiding this comment

Arkkimaagi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arkkimaagi left a comment

Choose a reason for hiding this comment

teroelonen commented Oct 26, 2023 •

edited

Loading