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.

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

1.7.0 #622

Draft
wants to merge 54 commits into
base: release-1.x
Choose a base branch
from
Draft

1.7.0 #622

wants to merge 54 commits into from

Conversation

ekes
Copy link
Member

@ekes ekes commented Sep 24, 2024

Release Notes

We have a number of new features in this release:

Sticky headers

If you want your site to have a sticky header when someone scrolls, you can check a checkbox now on the theme settings page. It has 3 options - not sticky, sticky, only sticky when scrolling.

Better focus order for header

When tabbing through the header, the focus order is not more suited for screenreader users. Especially when you tab into and then out of the services menu.

iOS Skip to Content link

We had a nasty issue which was very hard to replicate where keyboard users on iOS (iPhone, iPad) couldn't use the 'Skip to Content' link. Now they can.

Back to top

We have a checkbox now to switch on/off a "Back to top" button which will appear after you scroll more than the height of your screen

Accordion icons

Accordions now have a + icon when they are collapsed and a - icon when they are expanded

Print mode

When printing a step-by-step or guide page now the navigation will not be printed.

markconroy and others added 30 commits August 6, 2024 10:15
…lows

Use shared GitHub workflows to run tests
…b-token

Adds a variable to let us print a token for replacement in the breadcrumb trail.
This has several side-effects, all of them small:

- the aria-label attribute is lost
- prev/next links now wrapped in div.lgd-prev_next__label
- labels now say "Previous" and "Next" instead of "Previous Step" and
  "Next Step"
- labels are now bold
- distance between links and footer now *more* consistent with other CTs

Closes #565
Prior to this change, screen readers read "Next colon link". After, they
no longer pronounce "colon".

When title is set, the colon is included an spoken.
…utton-focus

adds better focus orders for services button/menu
…r_step_by_step_and_publications_prev_next

feat: replaces standalone prev-next code with included version
Note: this doesn't re-hide the link in circumstances where the viewport
size and size of content before #main-content mean the link is always
visible when #main-content scrolls into view.
Template now uses localgov_base _components/prev_next.twig.
…r-publications-prev-next

564 use prev next twig for publications prev next
markconroy and others added 11 commits September 23, 2024 14:12
ensures quick fact content is inside a <p> tag
adds CSS for blockquotes created via WYSIWYG
…teps-nav

hides guides/steps navigation in print mode
…llapse-to-accordion

feat: add expand/collapse icon to accordion
adds more space for clickable area for pager items
…ice-status-heading

fix: set color on service status heading
* initial commmit of sticky header settings for localgov_base

* moves class addition to preprocess_html

* sets default value

* check for setting before setting it

* renames support module to helper

* adds CSS + JS for sticky header

* rewrite to only use sticky header for logged out users

* update description for form item

* coding standards fixes

* removes duplicate code

* only apply the scroll check if the header type is set to scroll

* Add scroll-padding-top to the html and body when we have a sticky header

* coding standards fixes

---------

Co-authored-by: Maria Young <maria@agile.coop>
@ekes
Copy link
Member Author

ekes commented Sep 24, 2024

Is there a dependency on localgov profile that should be declared now?

@ekes ekes marked this pull request as draft September 24, 2024 12:06
…map-field

Add alt text to announce map link opens in a new tab
@ekes
Copy link
Member Author

ekes commented Sep 25, 2024

= Blocker breaker

To release this these MRs should be merged at the same time:

For LocalGov:

For LocalGov Microsites:

With these three #624 is resolved.

I have an open question if the tests will still pass, but that might just be my local #623

@ekes
Copy link
Member Author

ekes commented Sep 25, 2024

My notes about how to manually test (from Slack)

To review it all you'd have to:
(a) install LocalGov (3.0.10)
(b) upgrade the code of LocalGov to feature/740/enable-localgov_base-helper-module and LocalGov Base to 1.x
(c) Run the updb
(b) Make sure the localgov_base_helper module is enabled.
Then with the same code (LocalGov on feature/740/enable-localgov_base-helper-module and LocalGov Base on 1.x )
(a) install LocalGov
(b) Make sure the localgov_base_helper module is enabled.
Bonus for running phpunit on the test on localgov_base.
Next with microsites.
(a) install LocalGov Microsites (4.0.0)
(b) upgrade the code of LocalGov Microsites to feature/localgov-base-170 and LocalGov Microsites Base to feature/microsites-base-170 and LocalGov Base on 1.x
(c) Run updb
(d) Make sure the localgov_base_helper module is enabled.
Then with the same code (all those feature branches...)
(a) install LocalGov Microsites
(b) Make sure the localgov_base_helper module is enabled.
And finally a quick once over to check I wrote the composer.json version dependencies correctly.

@AWearring
Copy link
Contributor

@ekes
I was able to test (the first steps) on a copy of my local West Lindsey site and can confirm that localgov_base_helper is installed and enabled.

One thing I spotted on the "front end" is that #603 adds icons as expected, however this adds them to all sites even if they have overwritten the Twig template to introduce their own icons:
image
Whilst the addition of the icons is good I don't think it is wise that these are added in using JavaScript (we also add the button using this method too) - I have a PR (localgovdrupal/localgov_paragraphs#200) that @ctorgalson has reviewed and approved which moves the creation of the button and the addition of the icons into the Twig template which I think should be released along with localgov_base v1.7.0

@AWearring
Copy link
Contributor

@ekes
Also spotted these during composer update

Applying patches for drupal/preview_link

  https://www.drupal.org/files/issues/2024-05-28/3449121-9.patch (Add a 'copy to clipboard' feature for preview_link)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2024-05-28/3449121-9.patch

and
Applying patches for drupal/redirect

https://www.drupal.org/files/issues/2022-09-01/3057250-53.patch (Validation issue on adding url redirect: https://www.drupal.org/project/redirect/issues/3057250)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-09-01/3057250-53.patch

Looks like there are re-rolled patches available

@ekes
Copy link
Member Author

ekes commented Sep 26, 2024

@AWearring:

@ekes Also spotted these during composer update

Many thanks for the testing. I think both those patches are fixed in localgov (the profile) version 3.0.10

The #626 is a good catch and I think we want some resolution to it before pushing this out.

* Revert "adds checkmark icon"

This reverts commit e5ef66b.

* adds checkbox icon

* Add word-wrap to prevent overflow on .lgd-guide-nav__list

---------

Co-authored-by: Mark Conroy <mark@mark.ie>
Co-authored-by: Finn Lewis <finn@opencode.uk>
Co-authored-by: Stephen Cox <stephen-cox@users.noreply.github.com>
Co-authored-by: Andy Broomfield <andybroomfield@gmail.com>
@stephen-cox
Copy link
Member

The test failures I'm getting with this are:


1) Drupal\Tests\block_content\Functional\RegionRenderTest::testRegionRender with data set #0 ('disabled', 'FALSE')
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for localgov_base.settings with the following errors: localgov_base.settings:localgov_base_header_behaviour missing schema

/app/web/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:98
/app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
/app/web/core/lib/Drupal/Core/Config/Config.php:230
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:396
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:149
/app/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:75
/app/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:251
/app/web/core/includes/install.core.inc:1654
/app/web/core/includes/install.core.inc:695
/app/web/core/includes/install.core.inc:572
/app/web/core/includes/install.core.inc:121
/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:315
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:570
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:370
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

2) Drupal\Tests\block_content\Functional\RegionRenderTest::testRegionRender with data set #1 ('sidebar_first', 'TRUE')
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for localgov_base.settings with the following errors: localgov_base.settings:localgov_base_header_behaviour missing schema

/app/web/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:98
/app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
/app/web/core/lib/Drupal/Core/Config/Config.php:230
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:396
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:149
/app/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:75
/app/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:251
/app/web/core/includes/install.core.inc:1654
/app/web/core/includes/install.core.inc:695
/app/web/core/includes/install.core.inc:572
/app/web/core/includes/install.core.inc:121
/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:315
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:570
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:370
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

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.

10 participants