1207335 - Unify CSS for 'summary boxes'

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: Code Cleanup, enhancement)

Description

[Sebastian Zartner [:sebo]]

Currently for different summary boxes we use different CSS classes:

.cssprop
.htmlelt
.audionodebox
.summary-box-js
.summary-box-events

These should be merged to a single class.

Also see the discussion starting at bug 1106768 comment 7.

Sebastian

Comment 1

[Sebastian Zartner [:sebo]]

In bug 1106768 comment 15 Jeremie mentioned that we should better use tables instead of lists, so the class(es) should to address this.

Sebastian

Comment 2

[Jean-Yves Perrier [:teoli]]

Maybe we should set up a test page with the HTML we propose to use? (I'm a kind of a visual person)

Comment 3

[Sebastian Zartner [:sebo]]

Attachment: summaryBox.html

Well, the layout will not change, I guess.
Here's a simple test page demonstrating the current summary box being realized as a table.

Sebastian

Comment 4

[Stephanie Hobson [:shobson]]

Jeremie, could you provide an example of where you are already using tables?

The CSS code already supports his usage :)

Comment 5

[Stephanie Hobson [:shobson]]

Can we be more specific than summary-box or info-box for this class? property-definitions? syntax-summary?

Comment 6

[Sebastian Zartner [:sebo]]

Agree that summary-box and info-box are pretty general names, though they would still be fine IMO, as they summarize the information regarding a property, element, etc. property-definitions as a more specific name would be ok for me. syntax-summary seems wrong, as the information is unrelated to any syntax.

Sebastian

Comment 7

[Stephanie Hobson [:shobson]]

I went with .properties since it is shorter and more descriptive :) There's a pull request in.

This layout is formatted properly:

<table class="properties">
    <tr>
        <th scope="row">Applies to</th>
        <td>flex containers</td>
    </tr>
    <tr>
        <th scope="row">Inherited</th>
        <td>no</td>
    </tr>
</table>

Comment 8

[Sebastian Zartner [:sebo]]

Does the rendered layout still look the same?
My new 'cssinfo' template is ready[1] to generate the new structure.

Sebastian

[1] https://developer.allizom.org/en-US/docs/User%3ASebastianz/Test

Comment 9

[Jean-Yves Perrier [:teoli]]

We can try it by adding the hardcoded output directly on a test page.

Comment 10

[Sebastian Zartner [:sebo]]

Well, Stephanie's pull request[1] is not merged yet, so I can't see the changes.

Sebastian

[1] https://github.com/mozilla/kuma/pull/3543

Comment 11

[MDN Team (:mdn-dev)]

Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/a1265b1f974d62f3e9f2cbc11e9a1eefd8654430
Fix Bug 1207335 - Standardize summary-box code

Added new general class for the writers to transition all other instances too.

https://github.com/mozilla/kuma/commit/3fb4452677348b6e51d405ba5bc8cd3eed1ede76
Merge pull request #3543 from stephaniehobson/bug-1207335-rename-summary-box

Fix Bug 1207335 - Standardize summary-box code

Comment 12

[Sebastian Zartner [:sebo]]

I just realized that we have a rule with the selector

.audionodebox li li, .cssprop li li, .htmlelt li li, .properties li li, .summary-box-events li li, .summary-box-js li li

setting 'list-style-type: disc;'.

Because we only have one list with the new table structure, the rule does not apply anymore in cases like https://developer.mozilla.org/en-US/docs/Web/CSS/border-width.

Therefore the selector should be changed from '.properties li li' to '.properties li'.

Sebastian

Comment 13

[Jean-Yves Perrier [:teoli]]

Oh! Good catch. Can you file a specific bug and make it blocking this one?