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

textClassName property for Button component #4460

Closed
my-lalex opened this issue Dec 17, 2020 · 8 comments · Fixed by #6516
Closed

textClassName property for Button component #4460

my-lalex opened this issue Dec 17, 2020 · 8 comments · Fixed by #6516

Comments

@my-lalex
Copy link

my-lalex commented Dec 17, 2020

Environment

  • Package version(s): 3.36.0
  • Browser and OS versions: Chrome / macOS

Feature request

Would it be a good idea to add a 'textClassName' to the Button props, which would be added to the text span?

Examples

I'm currently creating a zone which can (or cannot) be a button, kinda
<Zone className='text-or-button' clickable={true}>Zone content</Zone>

The component is returning a <Button /> (with no margin, padding, background or border) or <span /> depending on the clickable prop.

The idea is to allow to customize the zone via CSS just by defining , without having to use .bp3-button-text

What do you think?

PS: Well done Palantir guys, Blueprint is a huge component set, very well designed and coded. And I love the highly customizable output thanks to CSS.

@adidahiya
Copy link
Contributor

Thanks for the kind words about Blueprint.

I'd like to avoid adding this prop unless absolutely necessary. I think you can work around this with CSS. If not, can you please create a code sandbox demonstrating the lack of customizability?

@my-lalex
Copy link
Author

my-lalex commented Jan 8, 2021

Of course you can work around with CSS (this is why I love working with the blueprint components), but the CSS would have to be "aware" of the .bp3-* classes hierarchy, which I try to avoid as much as possible...
The enhancement suggestion has for only purpose to be able to write cleaner CSS...

Let's see a pseudo-code

import { Button } from '@blueprintjs/core';
import { Component } from 'react';

export default class ClickableOrNot extends Component<{ clickable: boolean; className?: string }> {
	render() {
		const { clickable, className } = this.props;
		return clickable ? (
			<Button className={className}>{this.props.children}</Button>
		) : (
			<span className={className}>{this.props.children}</span>
		);
	}
}

<ClickableOrNot clickable={true} className="clickable-or-not-text">Text content</ClickableOrNot>

The CSS to have the same visual feedback would be

.clickable-or-not-text, .bp3-button.clickable-or-not-text > .bp3-button-text
	color: red

Wouldn't be more "elegant" to have ?

render() {
	const { clickable, className } = this.props;
	return clickable ? (
		<Button textClassName={className}>{this.props.children}</Button>
	) : (
		<span className={className}>{this.props.children}</span>
	);
}
.clickable-or-not-text
	color: red

@adidahiya
Copy link
Contributor

What happens if you change the color of the whole button? Why do you need to target the text span specifically?

@my-lalex
Copy link
Author

my-lalex commented Jan 8, 2021

Yeah, the color might be a bad example, but still significant if you use text/svg in your icon or rightIconproperties...

I'm thinking more about paddings, borders, etc...

@adidahiya
Copy link
Contributor

I'm inclined to decline this request for now. In general we guarantee backwards compatibility for the DOM structure rendered by Blueprint components, especially fundamental ones like buttons. So you should feel safe applying custom styling with .bp3-button selectors. You might want to parameterize that selector with the namespace, though, since we're going to change .bp3- to .bp4- in the next major version soon. (We do that in our own Sass code too.)

@my-lalex
Copy link
Author

my-lalex commented Jan 8, 2021

I get it. Thanks for answering ;-)

@adidahiya
Copy link
Contributor

Revisiting this old issue, I think should add a textClassName prop. This has been requested quite a bit to apply text overflow ellipsis styling to button text. Currently the only way to do this is something like this:

// in a CSS module

@use "@blueprintjs/core/lib/scss/variables" as bp;

.my-button {
 :global(.#{bp.$ns}-button-text) {
    overflow: hidden;
    text-wrap: nowrap;
    text-overflow: ellipsis;
  }
}

Whereas it would be nicer if users could do this:

<Button textClassName={Classes.TEXT_OVERFLOW_ELLIPSIS} />

N.B. it might be even nicer if Buttons had a prop for text overflow styling, but that's out of scope for this issue.

@adidahiya
Copy link
Contributor

Note that textClassName is also useful for muted text styling, which is currently not possible with simply:

<Button className={Classes.TEXT_MUTED} />

because this class loses on CSS specificity to some .bp5-button styles.

See #6516

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

Successfully merging a pull request may close this issue.

2 participants