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

News: updating news template #2458

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

News: updating news template #2458

wants to merge 1 commit into from

Conversation

Garneauma
Copy link
Contributor

@Garneauma Garneauma commented Dec 19, 2024 •
edited by duboisp
Loading

  • Updating paragraphs above and below the news table.
  • Adding instructions on what to do in basic HTML mode.

(wet-496)

@Garneauma
Copy link
Contributor Author

Pre-approved upon successful review and approval by the News team.

@duboisp duboisp self-assigned this Dec 30, 2024
@duboisp duboisp self-requested a review December 30, 2024 15:08
@Garneauma
Copy link
Contributor Author

Pre-approved upon successful review.

duboisp
duboisp approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Technically, this look good.

this is a Patch.

But prior to merge this, @Ricokola need to do some verification first.

duboisp
duboisp requested changes Jan 15, 2025
View reviewed changes
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Wait for @Ricokola check

Ricokola
Ricokola requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@Ricokola Ricokola left a comment

Choose a reason for hiding this comment

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

Pre-approved upon fixing a minor typo in news-en.html

templates/news/news-en.html
@@ -443,8 +449,7 @@ <h2 class="wb-inv">Search results</h2>
</tr>
</thead>
</table>
<p class="text-center nojs-hide"><a href="https://www.canada.ca/en/news/advanced-news-search/news-results.html">Access all news products</a>.</p>
Copy link
Contributor

@Ricokola Ricokola Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Please remove the extra dot at the end of "Access all news products".

@duboisp duboisp assigned Garneauma and unassigned duboisp Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ricokola Ricokola Ricokola requested changes

@duboisp duboisp duboisp requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@Garneauma Garneauma

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Garneauma @duboisp @Ricokola