callstack / react-native-paper

Material Design for React Native (Android & iOS)
https://reactnativepaper.com
MIT License
12.69k stars 2.07k forks source link

Allow Customization of Text in Button #658

Closed codypearce closed 5 years ago

codypearce commented 5 years ago

Current behaviour

Currently there is no way to change many properties for the text component in Buttons. For example, two properties which are causing issues for me at the moment are textAlign and margin. You can change some properties by wrapping the text in your own Text component, but this does not effect all the properties namely the two I mentioned.

Expected behaviour

Ability to change style of Text component in Button. This could be a prop on buttons like buttonTextStyle or something.

Code sample

This Snack shows no way to edit text-align or margin on the text component. https://snack.expo.io/r1Nz_Xoam

What have you tried

You can get around this by setting display: 'flex', justifyContent: 'flex-start', but similar to this https://github.com/callstack/react-native-paper/issues/352, the pressed state only appears over the text. Further, because I cannot change the margins on the text, I cannot change the padding from the side of the button to the text.

iphone x - 11 3 2018-11-15 10-13-53
ferrannp commented 5 years ago

Passing your own Text as children into Button would solve your problems?

codypearce commented 5 years ago

@ferrannp In the What you have tried section above I mentioned this solves some of the issues but not all of them. The snack above also has Text as a child of the Button component, as well as this new one so you can see more clearly https://snack.expo.io/Sytd8hE0m.

snack 2018-11-22 16-05-51

I need the button to be left aligned and I need to be able to change the margin or padding around the Text component inside the button which impossible at the moment, react-native-paper is adding padding/margin to whatever child is inside of the button component.

What I am suggesting would be a better solution is to be able to pass text style props to the Button component so all these properties can be adjusted by the user without having to wrap text in a Text component. If this solution is not something they would like to add, then simply being able to change the margin/padding on whatever child component that is in the Button component would still be much appreciated.

Trancever commented 5 years ago

@codypearce We are trying to strictly follow Material Design Guidelines and this Button that you'd like to use does not follow those guidelines. Of course, we could add few more props to make button component more flexible but on the other hand we want to keep the api as little as possible. One solution to your problem could be - copy all the button component's code from our library and adjust it to your needs, what do you think?

codypearce commented 5 years ago

@Trancever

We are trying to strictly follow Material Design Guidelines...

I can definitely respect this goal, however it is possible to both follow Material Design Guidelines strictly and provide flexibility for users who need something that deviates slightly. This wouldn't affect users who want to follow it strictly while providing a great benefit to those who need this flexibility.

Of course, we could add few more props to make button component more flexible but on the other hand we want to keep the api as little as possible.

I respect the goal for simplicity and flexibility is a trade off for simplicity in many cases. However, I would argue that the ability to style, and therefore customize, the majority of properties on any component is a fundamental/necessary feature for component libraries and it can be as simple as spreading a prop.

typography js netce-complete-mobile 2018-11-23 04-01-07

Which is what you're almost doing already

react-native-paper button js at master callstack react-native-paper 2018-11-23 04-05-10

...this Button that you'd like to use does not follow those guidelines...

This is true, this is what I need.

example

Which is a modified outline select from Material Design Guidelines https://material-components.github.io/material-components-web-catalog/#/component/select which is not technically not a button.

One solution to your problem could be - copy all the button component's code from our library and adjust it to your needs, what do you think?

I ended up creating a component using built in react-native components, however I think this is still a feature that is worth considering.

satya164 commented 5 years ago

Honestly the component you're trying to build looks so different from a button component that it doesn't make a lot of sense to use Button component as a base for that. The component that looks closest to what you have is the TextInput component, and you could probably achieve it by using the TextInput component.

which is not technically not a button

If you define buttons as things you click on, a lot of things can be considered as a button. But our button is supposed to strictly match the button component. Other things such as toggle buttons, FABs etc. have their own components even though they are technically buttons and they don't use Button as the base.

I'm not opposed to adding more customization options, but I want to see a good use case. The one here is not a proper use case for Button.

codypearce commented 5 years ago

@satya164

Honestly the component you're trying to build looks so different from a button component that it doesn't make a lot of sense to use Button component as a base for that.

This seems somewhat like an exaggeration, this is an dropdown component most close to the outlined Button. Though it is a "select" component almost all libraries use a Button component to create dropdown:

https://www.muicss.com/docs/v1/css-js/dropdowns Button used to active menu https://material-ui.com/demos/menus/ Buttons used to active menu https://materializecss.com/dropdown.html Custom component that looks like a button to active a menu https://getmdl.io/components/index.html#menus-section Buttons to active menu https://material-components.github.io/material-components-web-catalog/#/component/menu Button to active menu

The component that looks closest to what you have is the TextInput component, and you could probably achieve it by using the TextInput component.

This is what I initially tried but the <TextInput> component does not have onPress. As I mentioned above I used native react-native components to achieve this.

The one here is not a proper use case for Button.

Fair enough if that's how you feel, however I provided other reasons to allow this customization above which seems to have been missed. I can restate them here to make them more clear and separate from the original issue I was trying to solve.

  1. It is possible to both follow Material Design Guidelines strictly and provide flexibility for users who need something that deviates slightly.
  2. Some users need something that deviates slightly from Material Guidelines but still require such things as clickability, ripple effect, or shadow.
  3. The ability to style, and therefore customize, the majority of properties on any component is a fundamental/necessary feature for most component libraries. Most libraries accept a style prop to override whatever the library provides.
  4. This is a very simple addition and would not add any significant complexity to the api.

If this is not something you want to add, that's fine there are other solutions to this problem, I am simply proposing this as an enhancement to the current api at little cost.

@ferrannp @Trancever @satya164 I appreciate the time you've spent creating this library and responding to issues/comments.

satya164 commented 5 years ago

This seems somewhat like an exaggeration, this is an dropdown component most close to the outlined Button. Though it is a "select" component almost all libraries use a Button component to create dropdown:

This isn't an exaggeration. The screenshot you have posted is very different from Button. The text is left aligned, the outline is thicker and darker with a label on top left. I don't think it's possible to provide such flexibility without making the implementation very complex or exposing a lot of internal components which will make it harder to change those things later.

Yeah the component is called a button and technically buttons represent a lot of different styles of clickable elements, but it's not the goal of this component to be used as all of those. There's icon button, toggle button (wip), fab etc. which have separate components due to their difference in appearance even though they are technically buttons.

The linked libraries don't change any styling for the button. They just take the same button component and add a dropdown on top of it. If you wanted to implement the same functionality here, no changes to the button component would be required.

This is what I initially tried but the <TextInput> component does not have onPress. As I mentioned above I used native react-native components to achieve this.

The TextInput component has a render prop which allows you to render anything inside it, which I think will be perfect for this use case.

It is possible to both follow Material Design Guidelines strictly and provide flexibility for users who need something that deviates slightly.

Definitely. Though here I'd like to know what use cases will it address and then consider the API, instead of adding an API without knowing the use cases.

Some users need something that deviates slightly from Material Guidelines but still require such things as clickability, ripple effect, or shadow.

What is clickability? You can already customize shadow with the elevation style. Anyway, I'm not opposed to adding props, but I just want to know the use cases first.

This is a very simple addition and would not add any significant complexity to the api.

This looks simple on the surface. However, it's not as simple. A textStyle prop isn't sufficient and we need to consider several things:

There are probably more edge cases I haven't thought of. But it goes to show how adding a simple prop can affect several things. And even that won't help much with implementing your desired component. That's why I want to know the exact use cases when adding a feature so I can think about different scenarios and implement it properly.

In this case, I think using TextInput will be a much better choice for you, but ideally we'll have a dropdown component in future once we have a menu component.

Pushplaybang commented 5 years ago

I See this issue has been closed, and I've just hit the same wall. I think the case in question above, is too specific. @satya164 - I totally get the above mentioned goals, as well as those of material design.

Material design, is a set of principles and guidelines, and if we're not able to extend fundamental styles, then we're left with all our UI's looking like they were made at google. While thats not so bad for a prototype, it's not realistic for real work.

That said, the reason for using the library, would be to have the material foundation set. I there are a minimal number of extra props or options needed, to be parsed down the hierarchy, for certain components, for which the Button is a prime example. Buttons appear in all shapes and sizes, depending on their use case.

While I don't agree that you should have free-reign as mentioned above, I believe it's reasonable to have more control than what is currently available. Reimplementing the entire Button, which is complex already, on each project seems a little excessive.

in this example, the you can parse styles to the button, but If I adjust its size or padding, the touchable ripple/highlight remain unchanged. we should have a way to set the button height, width padding, etc.

If I've missed the boat on how to approach the above problem, please let me know, super keen to use RNP on my latest project.

satya164 commented 5 years ago

@Pushplaybang please open a new issue with your use case. There is a PR open already which should allow you to change the padding etc. #854