Skip to content
This repository has been archived by the owner on Mar 27, 2021. It is now read-only.

Render *Element components with div instead of span #61

Merged
merged 1 commit into from
Aug 21, 2017
Merged

Render *Element components with div instead of span #61

merged 1 commit into from
Aug 21, 2017

Conversation

indiesquidge
Copy link
Contributor

This is based on the confusion I had when trying to apply a custom class to one of these components.

@cweiss-stripe
Copy link
Contributor

cweiss-stripe commented Aug 18, 2017

Hi @indiesquidge, thanks for your PR. I am wondering why the span elements where confusing for you and if there are some specific difficulties in styling spans instead of divs?

The reason we went with them is, that spans are inline elements whereas divs are block elements. We decided to easily enable labels on the same line as the input elements by using span. In general inputs behave more like spans (inline) than divs, and we’re trying to mimic inputs a bit.

I’m closing this for now, but we can re-open if I am missing something crucial here.

@indiesquidge
Copy link
Contributor Author

I am wondering why the span elements where [sic] confusing for you and if there are some specific difficulties in styling spans instead of divs?

@cweiss-stripe, the link I included in the PR description outlined my confusion. I was attempting to apply custom styles to the element, but noticing that many of the styles were not taking effect.

In retrospect, this was more of an issue with me not realizing that <*Element> components rendered a span, but the point I bring up about it now is that rendering a span is actually more confusing than rendering a div because <*Element> components appear to render as block elements.

We decided to easily enable labels on the same line as the input elements by using span

Could you help me understand how using spans in the render enables the elements to have inline labels in this case? AFAICT, when I use any <*Element> component with a label, the component is rendered to the UI on a new line. Example

@indiesquidge
Copy link
Contributor Author

I believe this PR should be re-opened and reconsidered. Despite <Element> rendering a span, when the component is actually used, it is displayed in block format, which is more synonymous with the behavior of a div.

@cweiss-stripe cweiss-stripe reopened this Aug 21, 2017
@cweiss-stripe
Copy link
Contributor

cweiss-stripe commented Aug 21, 2017

(reopening this after looking into this further)

Thanks for the additional info!
Given the current behavior of Elements, which is indeed more synonymous with divs it seems more reasonable to use a div as a wrapper.
We'll change that and bump the react-stripe-elements version! 👍

@cweiss-stripe cweiss-stripe merged commit a6c82fc into stripe-archive:master Aug 21, 2017
@cweiss-stripe
Copy link
Contributor

Merged, thanks for the PR @indiesquidge

@indiesquidge
Copy link
Contributor Author

Awesome! Thank you, @cweiss-stripe

@indiesquidge indiesquidge deleted the element-render-div branch August 21, 2017 19:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants