Skip to content

fix: use spans inside button tag instead of divs#994

Merged
francoischalifour merged 2 commits into
algolia:nextfrom
lex111:button-div
Feb 2, 2021
Merged

fix: use spans inside button tag instead of divs#994
francoischalifour merged 2 commits into
algolia:nextfrom
lex111:button-div

Conversation

@lex111

@lex111 lex111 commented Feb 1, 2021

Copy link
Copy Markdown
Contributor

Summary

At the moment there is the following HTML issue related to the incorrect use of the button tag:

Element “div” not allowed as child of element “button” in this context. (Suppressing further errors from this subtree.)

I suggest replacing the button with a regular div element.

Result

According to the validator (https://validator.w3.org/nu/#textarea), the error has been fixed.

cc @francoischalifour

ref={ref}
>
<div className="DocSearch DocSearch-Button" {...props} ref={ref}>
<div className="DocSearch-Button-Container">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be moved to a span instead and the button stays (I think)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, we are more and more confident with this button tag.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree, it makes sense, replaced divs with spans.

@lex111 lex111 changed the title fix: avoid misuse button tag fix: use spans inside button tag instead of divs Feb 1, 2021

@Shipow Shipow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot Alexey and Haroen!

@Haroenv Haroenv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this looks like a solid fix, thanks!

@francoischalifour francoischalifour left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @lex111!

@francoischalifour francoischalifour merged commit f5c2a27 into algolia:next Feb 2, 2021
@francoischalifour

Copy link
Copy Markdown
Contributor

Released in @docsearch/react@3.0.0-alpha.33.

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.

4 participants