Closed amazingphilippe closed 2 years ago
Blocked as GC Articles are performing updates to WordPress
Updates done.
Okay, I reviewed most pages to make sure they looked okay and that the content was up to date, and published the ones that were. Here are the things I want to discuss, as I don't know if another round of front-end changes may be needed:
[x] Email formatting guide (EN + FR): seems to me that the output preview displays text too low (not aligned with top of box)
[x] Email formatting guide (EN + FR): the boxes act weird when we resize the window in preview mode. They move one above the other but with an offset.
[x] All pages: the margins are not the same as in Prod. (Text margins in prod are narrower)
[x] All pages: 2 columns in general - I used a 400 px width because that seems to match the text width. A 50% width doesn't match the text width though, it extends on all the screen width
[x] Security (EN + FR): the spacing before and after H3 is the same - it should be larger before H3 for better scanning.
[ ] Security (EN + FR): nested bullet points have the same look as primary bullet points.
[x] All pages: it seems to me that the titles font is not the same as before (it's currently different than the font used in prod)
[x] Fonctionnalités (FR): images are below the paragraph instead of being on the right.
[x] Features (EN): images are at correct position, but the alignment is too high in preview mode.
[x] Why GC Notify (EN + FR): Following the paragraph "Comply with communications requirements", the spacing between paragraphs is very large in preview mode, while there's no additional "Enter" in draft mode.
[x] Why GC Notify (EN + FR): The icons don't show up in preview mode.
[x] Misalignment in the footer (and need to remove Inclusive Language)
Also noting: the internal links can't be updated using the slug unless the pages are already published. So I think we'll need to go through all the pages to update the links once all pages are published.
@amazingphilippe Let me know if it doesn't make sense; I increased the estimate because there's a lot of tasks involved in this card, the a11y testing alone will be quite long to do!
~New Trello board for a11y testing: https://trello.com/b/HzI1ve9m/gc-articles-notify-public-pages~
EDIT: We'll use this existing a11y testing board. I created cards for GC Articles pages (blue cards at the top of the backlog).
Makes sense!
Partial route testing update (not all pages are published yet):
Terms of use EN: https://articles.cdssandbox.xyz/notification-gc-notify/terms-of-use/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/conditions-utilisation/
Privacy EN: https://articles.cdssandbox.xyz/notification-gc-notify/privacy/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/confidentialite/
Message delivery status EN: https://articles.cdssandbox.xyz/notification-gc-notify/message-delivery-status/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/etat-livraison-messages/
Personalisation guide EN: https://articles.cdssandbox.xyz/notification-gc-notify/personalisation-guide/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/guide-personnalisation/
Accessibility EN: https://articles.cdssandbox.xyz/notification-gc-notify/accessibility/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/accessibilite/
Guidance EN: https://articles.cdssandbox.xyz/notification-gc-notify/guidance/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/guides-reference/
May not be the best, but will serve our purpose for now - Spreadsheet for listing a11y issues
CSS fixes for this story: https://github.com/cds-snc/notification-admin/pull/1232
Ready to review
Élise continue this work today - updates in the linked links
New style changes required following PR #1232:
[x] Why Notify page: the issue of the icon being too high is resolved, however the spacing between the icon and the text is very large.
[x] All pages: The bullet points seem to have disappeared
@amazingphilippe Just want to check with you if you think we need other fixes or not:
The bullet points are not indented as they used to be (they're left aligned). I think it helps scanning when they're indented, but not sure.
The paragraph spacing in the tables changes when resizing the viewport, and there's still a lot of white space (but it's better than before). Normal viewport:
Narrow viewport:
Other new fixes in https://github.com/cds-snc/notification-admin/pull/1237, not merged yet.
Another partial update for route testing (for newly published pages)
Why GC Notify EN: https://articles.cdssandbox.xyz/notification-gc-notify/why-notify/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/pourquoi-notification/
Features EN: https://articles.cdssandbox.xyz/notification-gc-notify/features/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/fonctionnalites/
Security EN: https://articles.cdssandbox.xyz/notification-gc-notify/security/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/securite/
Formatting guide EN: https://articles.cdssandbox.xyz/notification-gc-notify/formatting-guide/ FR: https://articles.cdssandbox.xyz/notification-gc-notify/fr/guide-mise-en-forme/
~The only remaining page to publish in the home page, but we need to confirm with the GC Articles team which page is the correct one, and that the styles are okay.~
EDIT: The home page is now published and the styles look fine, but there's an issue with having the same url in EN and FR (because it's just the domain). I'm looking into how we could add /home and /accueil with the GC articles team, because GC Articles doesn't make it possible to change the permalink for the front page.
Elise working on a11y testing today, needs dev help on one page from GC Articles team, have to add possibility to add permalink for bilingualism
Update on home page url: creating a FR home page will be on the Notify routing side. Link to new issue for this
Not working on this card today so moving back into sprint backlog.
On removing routes overriding GC Articles routes
Had a chat with Jimmy - it may be a task more involved than it seems and the devs should look at all potentialities because urls kind of touch to the "nervous system" of Notify. We agreed that I'll create a PR to get an instance running and allow for testing, and I'll also create a new card specifically for that task.
Marking this card as currently blocked since we need dev work to make the routes work. Link to Zenhub card for that work: Remove existing routes using same url as routes in GC Articles
Link to PR to set off this work: Remove routes that override GC Articles routes #1248
Also marking the card blocked by other dev work that needs to be done to make the home page bilingual.
Sync with Phil C, Paul, Élise and devs to sync on goals for this work Gather knowledge across teams to discover real issue we're trying to fix and solutions.
Decision to scope this work down to keeping URLs the same Set up GC articles to have the same URLS we currently have for now No bilingual routes Stop further work on Google Analytics and Google tag Manager for now, pick up those cards later
Wordpress doesn't accept unilingual slugs, Phil looking into this today
The PR introduces two ways in which in can finish this task. Looking for some code review, its very repetitive and I think we can make it less bad code.
Phil is doing 2 things:
Paired with @amazingphilippe / @sastels and we've come up with some new code to help cut down on the repetition.
Not losing any formatting on pages a11y scans on published links Can test again ally scan in staging before pushing to prod
@yaelberger-commits Will get a status update from GC Articles team on caching PR and then hand back to Phil once that blocker has been removed
Jimmy and Andrew reviewed GC Articles caching PR, made some comments, team making some changes, Jimmy and Andrew to hopefully take another pass today.
The GCArticles caching feature was merged and integrated into the mainline. That should not be a blocker anymore. Phil and Dave are meeting to sync on latest changes and know what's the best way forward on this task.
Re linking the routes in a new PR: https://github.com/cds-snc/notification-admin/pull/1275
Review has been done, but want to reassess a change. Needs a few hours of work left
Hierarchy of templates was changed with GC Articles. Lost a few elements along the way.
Functionally complete, Elise will test that content is coming through ok
Checks from @EliseKa to see if this is good to go
During our test hack session, the following issues were found:
/message-delivery-status
pointing to staging/security
page
Test content created using GC Articles as CMS
Description
As a Notify team member, I need to be able to make sure the content created in GC Articles is working, bug free and accessible so that I can complete the content migration.
WHY are we building? Easy to maintain content without interfering in the Notify code. WHAT are we building? Some content pages in GC Articles VALUE created by our solution. Easy to maintain content without interfering in the Notify code.
Acceptance Criteria** (Definition of done)
Given we have drafted content in GC Articles, when we are ready to publish, then we are sure to release a bug free, accessible content update.
Things to do and figure out before we release the GC Articles collab work:
QA