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

[12.x] Fix styles overwriting checkout button when class is set #1070

Merged
merged 7 commits into from
Feb 22, 2021
Merged

[12.x] Fix styles overwriting checkout button when class is set #1070

merged 7 commits into from
Feb 22, 2021

Conversation

natecorkish
Copy link
Contributor

@natecorkish natecorkish commented Feb 21, 2021

Fixes #1069

If the class is set, the styles would overwrite the class styles, because we're showing default styles because class isset.

Now you can do something like:

{{ $checkout->button('Pay For Job', [
    'class' => 'bg-blue-100 text-sm px-4 py-2',
    'styles' => 'other styles here that don't clash with classes.'
]) }}

@natecorkish natecorkish changed the title Fix styles overwriting checkout button when class is set [12.x] Fix styles overwriting checkout button when class is set Feb 21, 2021
@taylorotwell
Copy link
Member

To me the behavior I would expect would be to only show these default styles if NO style is passed and NO class is passed by the developer. If either of those things are passed we can't safely add these default styles. /cc @driesvints

@taylorotwell
Copy link
Member

In other words:

style="{{ ! isset($style) && ! isset($class) ? 'background-color:#6772E5;color:#FFF;padding:8px 12px;border:0;border-radius:4px;font-size:1em' : ''; }}"

@natecorkish
Copy link
Contributor Author

In other words:

style="{{ ! isset($style) && ! isset($class) ? 'background-color:#6772E5;color:#FFF;padding:8px 12px;border:0;border-radius:4px;font-size:1em' : ''; }}"

That makes sense. Updated the commit.

@driesvints
Copy link
Member

driesvints commented Feb 22, 2021

The way the current PR is shaped would cause the $style attribute not to be applied at all. You probably still need to add it in the same fashion as $class.

In any case we need to be aware that this is technically a breaking change for anyone using the current behavior. If anyone was either adding styling with the $class not set and the $style set and also relying on the default styling to be present they'll see their buttons broken.

I do agree that the new behavior makes more sense.

@natecorkish
Copy link
Contributor Author

natecorkish commented Feb 22, 2021

The way the current PR is shaped would cause the $style attribute not to be applied at all. You probably still need to add it in the same fashion as $class.

In any case we need to be aware that this is technically a breaking change for anyone using the current behavior. If anyone was either adding styling with the $class not set and the $style set and also relying on the default styling to be present they'll see their buttons broken.

I do agree that the new behavior makes more sense.

Updated behaviour. Can now do something like:

{{ $checkout->button('Pay For Job', [
    'styles' => 'color: #fff'
]) }}

or

{{ $checkout->button('Pay For Job', [
    'class' => 'text-red-50'
]) }}

or

{{ $checkout->button('Pay For Job') }}

If style isset, it'll use custom style, if no class, and no style is set, it'll use default style and If only class isset, will use that.

@driesvints
Copy link
Member

In any case in the next major update we'll be able to implement a proper Blade component since we're dropping Laravel 6 support.

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.

Checkout button, cannot change style if class is set, and if class is set, styles overwrite class.
3 participants