Skip to content

[List] Fix w3c issues#10050

Merged
oliviertassinari merged 2 commits intomui:v1-betafrom
oliviertassinari:w3c-list
Jan 27, 2018
Merged

[List] Fix w3c issues#10050
oliviertassinari merged 2 commits intomui:v1-betafrom
oliviertassinari:w3c-list

Conversation

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jan 26, 2018

I have fixed a lot of errors 🎉 :

  • [Dividers]:
    Error: Element “hr” not allowed as child of element “ul” in this context.
    Reason: hr is used as a direct chid of ul (I am not 100% sure if this is in component source or demo/docs only)
  • [Lists]:
    Error: Bad value “button” for attribute “role” on element “li”
    Reason: May be demo error only.

It's tricky to get this right. it's related to #9867

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. scope: list Changes related to the list labels Jan 26, 2018
@oliviertassinari oliviertassinari self-assigned this Jan 26, 2018
@oliviertassinari oliviertassinari force-pushed the w3c-list branch 2 times, most recently from 1b64909 to 6570fde Compare January 27, 2018 00:13
@mbrookes mbrookes self-requested a review January 27, 2018 00:15
@oliviertassinari oliviertassinari removed their assignment Jan 27, 2018
Copy link
Member

@mbrookes mbrookes left a comment

Choose a reason for hiding this comment

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

Great work! Just a few minor nit-picks.

## List Dividers

The divider renders as a `<hr>` by default.
You can save this DOM element by using the `divider` property on the `ListItem` component.
Copy link
Member

Choose a reason for hiding this comment

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

"save rendering this DOM element"

Copy link
Member

Choose a reason for hiding this comment

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

You could perhaps add an example of that to the List demos (or revise an existing one), and link to it from here.

## Inset Dividers

The following example demonstrates the `inset` property.
We need to make sure the `Divider` is rendered as a `li` to match the HTML5 specification.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add: "The example shows two ways of achieving this."


/**
* `ButtonBase` contains as few styles as possible.
* It aims to be a building block for people who want to create a simple button.
Copy link
Member

Choose a reason for hiding this comment

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

"It aims to be a building block for creating a simple button."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: list Changes related to the list type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants