fix(page-notice): fixed paragraph-only positioning

[ArtBlue]

Fixes #1868

• This PR contains CSS changes
• This PR does not contain CSS changes

Description

I fixed notices which only had a <p> and no <h2>.

Screenshots

Before:

After:

Checklist

• I verify the build is in a non-broken state
• I verify all changes are within scope of the linked issue

• I regenerated all CSS files under dist folder
• I tested the UI in all supported browsers
• I did a visual regression check by doing a Percy build and approved the build
• I tested the UI in dark mode and RTL mode
• I added/updated/removed Storybook coverage as appropriate

[agliga]

I believe section notice and inline notice both have their icons misaligned still. We should try to fix those as part of this PR
I saw that inline notice had a 4px top/bottom margin with a paragraph but it looks pretty off when aligning with an icon.

[ArtBlue]

I believe section notice and inline notice both have their icons misaligned still. We should try to fix those as part of this PR I saw that inline notice had a 4px top/bottom margin with a paragraph but it looks pretty off when aligning with an icon.

I think that makes sense for inline-notice (it already has that use case with just a <p>) since it's inline and does not denote a hard section break (I'm fixing that one), but for page notice and section notice, should we really be promoting the use of the notice with just a <p> and no heading? This is the current structure:

<div class="section-notice section-notice--attention" role="region">
    <div class="section-notice__header" role="region" aria-roledescription="Notice">
        <svg aria-hidden="true" focusable="false" class="icon--attention-filled-small">
            <use href="#icon-attention-filled-small"></use>
        </svg>
    </div>
    <div class="section-notice__main">
        <h3 class="section-notice__title">Add required aspects to keep item live.</h3>
    </div>
    <div class="section-notice__footer">
        <button class="fake-link">Dismiss</button>
    </div>
</div>

This would need to be the structure variation to support that change:

<div class="section-notice section-notice--attention" role="region">
    <div class="section-notice__header" role="region" aria-roledescription="Notice">
        <svg aria-hidden="true" focusable="false" class="icon--attention-filled-small">
            <use href="#icon-attention-filled-small"></use>
        </svg>
    </div>
    <div class="section-notice__main">
        <p>Add required aspects to keep item live.</p>
    </div>
    <div class="section-notice__footer">
        <button class="fake-link">Dismiss</button>
    </div>
</div>

It looks to me that styling like this was to accommodate normal styling (looks like <p>) for standalone <h2/h3> tags:

.page-notice__title {
    font-size: @font-size-regular;
    font-weight: normal;
    margin: 4px 0 0;
}

/* legacy version with separate bold heading */
.page-notice__title:not(:only-child) {
    font-weight: bold;
}

I actually had some hesitations about even making the fix for the page notice. So, I guess the question is do we want to promote the usage of the page/section notice with just the <p> as it's semantically not as good as a heading? Or is it trivial enough to cover the use case and not worry about it?

@ianmcburnie , any thoughts on this?

[agliga]

A quick look at figma shows that we do support no title in section notice

[ArtBlue]

Hmm.. does Figma really help answer the question though? I think that is way too upstream and limited. I don't think design can/should prescribe the actual tags we use, just the way things look. My question has to do more with the actual tags - semantics and a11y. Figma's specs of "without" title doesn't really take the actual tags into consideration; not sure it answers the question. We have headings we make look like untitled variations.

[agliga]

Hmm.. does Figma really help answer the question though? I think that is way too upstream and limited. I don't think design can/should prescribe the actual tags we use, just the way things look. My question has to do more with the actual tags - semantics and a11y. Figma's specs of "without" title doesn't really take the actual tags into consideration; not sure it answers the question. We have headings we make look like untitled variations.

Not sure if I understand correctly.
If we use a heading, it should look like a "title" and then below that we have a paragraph for more content. If we remove the title, we only have a paragraph content. I don't really agree with having the "heading" also support no title case. That would be a larger change since we have to have a modifier to unbold the heading.

[ArtBlue]

To me, this seems to be saying that a heading is always needed because it makes an affordance for when it's alone:

.page-notice__title {
    font-size: @font-size-regular;
    font-weight: normal;
    margin: 4px 0 0;
}

/* legacy version with separate bold heading */
.page-notice__title:not(:only-child) {
    font-weight: bold;
}

And when it's the only thing in the container, it gets treated more like a <p>. So, if we can just have a <p>, why not just have a <p> and not force the <h2> to look like a <p>? But if a heading is always needed, why support only a <p>?

[agliga]

To me, this seems to be saying that a heading is always needed because it makes an affordance for when it's alone:

.page-notice__title {
    font-size: @font-size-regular;
    font-weight: normal;
    margin: 4px 0 0;
}

/* legacy version with separate bold heading */
.page-notice__title:not(:only-child) {
    font-weight: bold;
}

And when it's the only thing in the container, it gets treated more like a <p>. So, if we can just have a <p>, why not just have a <p> and not force the <h2> to look like a <p>? But if a heading is always needed, why support only a <p>?

I believe that legacy version is for ds4. So it probably shouldn’t be relied on.
Regardless this issue was filed because teams are using a p element alone.
We can discuss about restructuring the notice to not support a p tag next major version if we like. But for now it seems like this is causing issues for teams and we should fix it. We should also make sure that we cover all the other cases so this doesn’t come up again.