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

Empathy Thematic - Improve CSS for Card Pattern #2445

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

Conversation

BrahimMahadi
Copy link
Contributor

The following pull request was made to address CSS styles that have been injected to the landing page of the What to do when someone dies section using the Multi-Media component instead of having these styles added to the existing Empathy theme.

General checklist

  • Implementation of the injected CSS classes to the existing Empathy theme.
  • Tested and ensured styles are properly implemented.
  • Adjusted documentation to reflect new classes
  • Reference JIRA ticket WET-493

@Garneauma
Copy link
Contributor

Pre-approved upon successful review.

@Garneauma Garneauma self-assigned this Nov 18, 2024
@Garneauma Garneauma self-requested a review November 18, 2024 15:30
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

In addition to the requested changes. Please include in the meta.md the following:

  • Clear and simple explanations of the new feature, change or update;
  • The impact on the sponsored department of the new feature, change or update;
  • The impact on the public of the new feature, change or update.

You can refer to the "changes" property in the meta.md file of 2024-10-datatable-utilities.

@@ -92,3 +92,111 @@ <h4>Code</h4>
<pre><code>&lt;div class="wb-frmvld provisional wb-steps quiz empathy panel-body"&gt;
&lt;!-- Steps Quiz Form --&gt;
&lt;/div&gt;</code></pre>

<h3>Life Journey Home Page Layout</h3>
<div class="mrgn-tp-lg container">
Copy link
Contributor

Choose a reason for hiding this comment

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

Either remove this container or use the page layout without container and add containers where needed.

Suggested change
<div class="mrgn-tp-lg container">
<div class="mrgn-tp-lg">

<div class="row wb-eqht-grd">
<div class="col-sm-12 col-md-6 mrgn-bttm-lg">
<div class="empathy cards h-100 full-width">
<a href="#" class="stretched-link"><h2 class="h4">Preparing for end of life</h6></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Anchors should be inside headings not around them (do for all items). Also, this one has an closing tag instead of .

Suggested change
<a href="#" class="stretched-link"><h2 class="h4">Preparing for end of life</h6></a>
<h2 class="h4"><a href="#" class="stretched-link">Preparing for end of life</a></h2>

Copy link
Member

Choose a reason for hiding this comment

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

Well, that heading can't be an heading level 2, it must be an heading level 4

If you absolutely need to provide an example that show a heading level 2, you can add a new page only for that example.

<a href="#" class="stretched-link"><h2 class="h4">Preparing for end of life</h6></a>
<p>Information on how to prepare, organize your affairs and leave instructions to your survivors.
</p>
<span class="fas fa-arrow-right" aria-hidden="true"></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the Fontawesome CSS to the page in order for these arrows to show up.

</div>
</div>
<h4>Code</h4>
<pre><code>&lt;div class="mrgn-tp-lg container"&gt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the following div to the top of the page to style the code sample.

Suggested change
<pre><code>&lt;div class="mrgn-tp-lg container"&gt;
<div class="wb-prettify all-pre hide"></div>
<pre><code>&lt;div class="mrgn-tp-lg container"&gt;

.empathy.quiz-img {
width: 100%;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you are missing the custom styles for the <details> elements on the results page after the quiz. Please make sure all the custom styles for all empathy pages are taken into account.

<div class="row wb-eqht-grd">
<div class="col-sm-12 col-md-6 mrgn-bttm-lg">
<div class="empathy cards h-100 full-width">
<a href="#" class="stretched-link"><h2 class="h4">Preparing for end of life</h6></a>
Copy link
Member

Choose a reason for hiding this comment

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

Well, that heading can't be an heading level 2, it must be an heading level 4

If you absolutely need to provide an example that show a heading level 2, you can add a new page only for that example.

<h3>Life Journey Home Page Layout</h3>
<div class="mrgn-tp-lg container">
<div class="row wb-eqht-grd">
<div class="col-xs-12 col-md-8 empathy main-content small">
Copy link
Member

Choose a reason for hiding this comment

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

What mean "main-content"? Can you use a more meaningful name? That section looks like to be a "Service and information" similar section. Am I right? If so can you use a similar name.

<div class="col-sm-12 col-md-6 mrgn-bttm-lg">
<div class="empathy cards h-100 full-width">
<a href="#" class="stretched-link"><h2 class="h4">Benefits and Programs</h2></a>
<p>Applying for benefits to which you may be entitled following a death.</p><br>
Copy link
Member

Choose a reason for hiding this comment

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

Why a <br> here and not everywhere. What is the use? If it is to position the font-awesome icon it will be preferable to put it into its own block element that don't convey any semantic, like a <div>

Suggested change
<p>Applying for benefits to which you may be entitled following a death.</p><br>
<p>Applying for benefits to which you may be entitled following a death.</p>

<p class="mrgn-tp-lg"><span class="fas fa-clock"></span> This will take less than a minute.</p>
<a href="#" class="mrgn-tp-lg btn btn-default btn-empathy btn-block btn-lg">Start questionnaire</a>

<img class="quiz-img mrgn-tp-lg center-block hidden-xs hidden-sm" src="./images/lloj-direction-cloud.256.png" alt="Someone died, what do I do?">
Copy link
Member

Choose a reason for hiding this comment

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

The image don't load when I run this change in my local environment.

Is that image decorative or not? It seems to be decorative when I look at the inserted image.

*/
@media screen and (max-width: 991px) {
.empathy.main-content {
order: 1;
Copy link
Member

Choose a reason for hiding this comment

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

That is problematic and make the content failing WCAG regarding the meaningful order Success Criteria in small device.

Suggested change
order: 1;

Copy link
Member

Choose a reason for hiding this comment

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

Note that images are not copied over and are not included in the thématique library.

Here to see the issue is the image is not available in the compiled version of the thématique, the one linked from our méli-mélo page which is locally located here: /méli-mélo-démos/gc-thématique/th-empathy/

@Garneauma Garneauma assigned BrahimMahadi and unassigned Garneauma Nov 25, 2024
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.

3 participants