React-dates: Class name prop for portal

[drobannx]

I'm running into a styling issue when using the date picker with the portal option. The containing div for the portal does not have a class name or a way to provide a class name. If there was a prop exposed to pass in a portalClassName, this would allow me to style accordingly.

If this is something that would be accepted, I'd be happy to look into submitting a PR to add this functionality.

Thanks!

[lencioni]

Hi @drobannx! Thanks for the suggestion. However, we'd prefer to avoid allowing classNames to be passed between components. More info: https://brigade.engineering/don-t-pass-css-classes-between-components-e9f7ab192785

[drobannx]

Thanks for the quick response! I completely understand about the added complexity of doing that. I looked into react-portal supporting class names and they removed that for the 3.0 release.

My current dilemma is we have to support some dynamic theming, so we are using CSS-in-JS which has made this requirement easy to update theming on the fly. But using the withPortal or withFullScreenPortal prop, I am not able to control the styling of the date picker once it is in the portal. Is there a way to accomplish this that I'm missing? And if not, do you have any suggestions? Thanks so much for your time!

[ethanresnick]

@lencioni I have a similiar need, and I read the linked article, but none of its reasoning seems to apply to publicly-distributed/third-party components. Sure, if I'm authoring a component, I can add explicit properties that denote the different axes of variation. But, if I'm using a third-party component off the shelf, that third party can't predict all the variation I may need and needs to offer some way for me to customize things. Right?

React Dates offers the sass variables, of course, but those are really just like a fixed set of props in the sense that, if there's no Sass variable for some type of variation, there's no way for me to style it. That, in my mind, points to: every element should have a class name, so I can customize the look along every css property. Moreover, every (major) element should allow me to provide a custom class name so that I can create BEM-style variants.

[ethanresnick]

@lencioni Unrelated, I just want to say thank you for all your open source work (beyond this Airbnb-affiliated stuff). I've bumped into, and gotten great value out of, a number of your projects over the years, going back to my early days as PHP programmer when I used something you wrote for (I think it was) image resizing or script concatenation.

[ljharb]

I'd say the third party component shouldn't try to be customizable; if you want something it can't explicitly handle, file an issue of make a PR and it can add it. Use cases it doesn't know about are ones it can't support.

[ethanresnick]

I'd say the third party component shouldn't try to be customizable; if you want something it can't explicitly handle, file an issue of make a PR and it can add it.

I've been thinking about this recently, so I have some working ideas below that I'd love to hear both of your thoughts on.

My suspicion is that component users often need a huge amount of variation for a component to be usable, and that it would be impractical for component authors to add switches for all those things. (That's the argument for having @apply in the web components world.) React-dates adds classnames to most of the elements it renders, so I suspect users have taken that as an invitation to add their own styles that way; I know I did, as did the OP. It's only when those class names are insufficient that issue reports come in. Without any class names, I imagine there'd be a lot more people on here asking for all kinds of forms of customizability.

To give a concrete example, the actual use case that led me to this issue was that I needed to change the z-index of just one instance of this library's SingleDatePicker component. I would have liked to pass a custom id/classname to that instance, but couldn't. (I ended up targeting the .DatePicker class that react-dates already exposes, using parentSelector .DatePicker, but that's not ideal.) To address this use case, should all of the components in this library support a zIndex property? I guess you could say yes, but then (to repeat) I suspect you're going to find yourself adding 10 other switches too, if you're catering to the wider world.

Also, I want to point out a practical issue: had I needed to give my date picker different z-indexes at different media queries, the only way for that to work would be for the component to let me provide a custom id/classname; a static zIndex prop wouldn't suffice. The example along these lines that really persuades me is font-size/leading: if a component renders any text, users are going to want to fit that text into their site's typographic hierarchy; that hierarchy is likely to be responsive, so users are going to need to be able to give the component's text different font-sizes/line-heights at different media queries. And, again, a prop (or sass variable) won't suffice for that. So, if different font-sizes at different media queries is a legit use case for third-party components, custom classes/ids seem to be the way to go, for now anyway. (I needed this responsive typography myself, and so overrode react-dates' .DateInput styles.) This will all change once IE dies and we can use css custom properties, at which point the component could just take a custom property name in its zIndex/fontSize prop, but we're not there yet.)

At the end of the day, the goal is to have enough extensibility that users can do what they need while that extensibility is controlled enough that the component author can update their components without breaking someone's added styles. However, I just don't think our tools and community conventions are powerful enough yet where we can create components that actually accomplish both of those things. (In fact, most of our components probably accomplish neither.) And, when we have to pick between offering consumers more freedom for customization and more protection from internal internal library changes, I lean toward offering more customization, for two reasons:

1. As a consumer, I always have the relatively painless option to not update a component, or to update it only after testing the new version. However, if I start using a component and only later find out that, for one of my use cases, it's not sufficiently customizable, that's a much bigger problem: it may be cumbersome to switch by then, and its bad for performance, and possibly UX consistency, to add a second component with very similar functionality just for one case.

2. I'm skeptical of the idea that it's even possible, with our current limited tools (e.g., no shadow dom, css custom properties), for an author to change their component without breaking some consumers, so I think the consumers-updating-cautiously approach described above is kinda inevitable anyway. Personally, I know I have had to depend on react-date's existing CSS and DOM structure in many ways (which I'm happy to detail if your curious), even though I try to be conscientious about this stuff, and that my code would break if react-dates were to be refactored, even if all its components kept their public (props-based) API the same.

Thoughts on this?

[ljharb]

In the case of z-index, you'd definitely want a static prop, but you'd use matchMedia in a flux store or a container to rerender the component based on the breakpoint.

It's highly possible to change a component without breaking consumers; which is exactly why it's so important to start out with a tightly constrained API - the more things you support (explicitly or via customization) the harder it is to avoid breakage.

In general, you can always wrap any React component in a <div> that you style any way you like, and this can sidestep many issues around z-index, media queries, typography, etc.

[ethanresnick]

Let's hold aside z-index for a second, because you're right that a reasonable approach to that is a wrapper div with the z-index set there.

It's highly possible to change a component without breaking consumers; which is exactly why it's so important to start out with a tightly constrained API

Yes, of course, it is possible to avoid breakage with a constrained API. My point was that such an API will probably be too constraining for the component to be useful to many, many people, and those people will be doing a reasonable thing when they circumvent the API, at which point we're back to breakage. Compared to traditional software modules, I think UI components have more degrees of legit variation, and our tools for expressing that are less powerful, which leads me to conclude that hammers like custom class names are reasonable for now.

To make this concrete: react-dates is a very robust component compared to most of the ecosystem; it's mature, and developed by smart people. Yet there were still 5(!) unsupported customizations that I had to make in order to use the SingleDatePicker component in my pretty simple site. Given that, what should I have done? Opened 5 issues and waited indefinitely hoping they'll be resolved? Submitted PRs, which still might not be merged any time soon and would've required making changes across the library, even in components I wasn't using? Maintained a custom fork? All of that's a lot of work, and I was on a deadline... So, instead, I overrode a few of the library's built-in styles, which worked well and has been easy to maintain—but was only possible because the library put a lot of class names on things that I could latch onto as unofficial extension points. The ability to do that should be seen as a feature, not a bug.

Cf. this issue and this comment.

[ljharb]

It's worth noting that regardless of what you did, you absolutely should have opened 5 issues.

The negative effect you're missing is, that by silently customizing it to meet your needs, you benefited, and you also deprived the project of benefiting from knowing about your use cases, and perhaps providing explicit/supported/superior means of achieving them.

[ethanresnick]

It's worth noting that regardless of what you did, you absolutely should have opened 5 issues.

This is 100% fair, and opening issues seems like the least I can do to give back. I was under intense time pressure at the time, but I'll open the issues now.

[ethanresnick]

Ok, I've opened #533, #534, #535, #536, #537, and #538. Most of them are variations on #78, but they're a little more specific, so hopefully that's helpful.