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

fix(selectable-tile): change divs inside label to span #4745

Merged
merged 5 commits into from
Nov 25, 2019
Merged

fix(selectable-tile): change divs inside label to span #4745

merged 5 commits into from
Nov 25, 2019

Conversation

abbeyhrt
Copy link
Contributor

Closes #3369

According to the issue, it is invalid HTML markup to have a div inside of a label so I changed them to span

Changelog

Changed

  • div->span

Testing / Reviewing

Make sure the SelectableTile looks the same and is still functioning properly

@abbeyhrt abbeyhrt requested a review from a team as a code owner November 21, 2019 19:25
@ghost ghost requested review from emyarod and joshblack November 21, 2019 19:25
@netlify
Copy link

netlify bot commented Nov 21, 2019

Deploy preview for the-carbon-components ready!

Built with commit 38ef59c

https://deploy-preview-4745--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Nov 21, 2019

Deploy preview for carbon-elements ready!

Built with commit 38ef59c

https://deploy-preview-4745--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Nov 21, 2019

Deploy preview for carbon-components-react ready!

Built with commit 38ef59c

https://deploy-preview-4745--carbon-components-react.netlify.com

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @abbeyhrt! Just curious, does the same thing apply to single-selectable tile?

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

<RadioTile> probably needs this change as well right?

@abbeyhrt
Copy link
Contributor Author

@asudoh maybe but I'm not sure what you mean by "single-selectable tile", could you clarify for me?

@abbeyhrt
Copy link
Contributor Author

@emyarod yeah it does! I'll make that change. 👍

@asudoh
Copy link
Contributor

asudoh commented Nov 23, 2019

@abbeyhrt Would you try searching for RadioTile in the codebase? Thanks!

@asudoh
Copy link
Contributor

asudoh commented Nov 25, 2019

Thanks @abbeyhrt for the update!

@asudoh asudoh merged commit d7ac8fa into carbon-design-system:master Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selectable Tile: Invalid HTML Markup
4 participants