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

Feature: bind to style with object, array and string merging/override support #236

Closed
wants to merge 5 commits into from

Conversation

HugoDF
Copy link
Contributor

@HugoDF HugoDF commented Mar 5, 2020

Closes #213 Adds Vue-style binding support for the "style" attribute.

Style binding supports:

  • string definition with merging/override support
    • eg. <div style="display: inline" :style="'padding: 10px'"></div> -> display: inline; padding: 10px applied
  • array definition with merge/override support
    • eg. <div style="display: inline" :style="['display: hidden; width: 200px;', 'height: 20px;']"></div> -> display: hidden; width: 200px; height: 20px;
  • object definition with override support
    • eg. <div style="padding-top: 0" :style="{display: 'hidden', paddingTop: '15px', borderColor: 'teal'}></div>" -> display: hidden; padding-top: 15px; border-color: teal

Update README with style object/array support.

Unrelated change (to tests): fixes missing assertion calls in class bind tests (see 6d9e2a0), I'm happy to submit this as a separate PR. Submitted as #360.

@HugoDF HugoDF changed the title Feature: bind to style with object, array and string merging support Feature: bind to style with object, array and string merging/override support Mar 5, 2020
@HugoDF HugoDF force-pushed the feat-style-object branch 3 times, most recently from 7c0edb4 to a464b5c Compare March 5, 2020 21:03
@HugoDF
Copy link
Contributor Author

HugoDF commented Mar 7, 2020

@calebporzio this has been update with latest master changes and is ready for you to check out 👍

@HugoDF
Copy link
Contributor Author

HugoDF commented Mar 22, 2020

@calebporzio any idea when this PR might be integrated? I'm aware it's a chunky one but it does bring support for a feature you mentioned just not having time to tackle.

@atomgiant
Copy link
Contributor

I would also really like to see this functionality in Alpine. In my case I am using it for a color picker to dynamically adjust the selected color background. Here is a CodePen which shows it not working if that helps https://codepen.io/atomgiant/pen/YzyzYxd

@HugoDF
Copy link
Contributor Author

HugoDF commented Apr 9, 2020

I've cleaned this PR up for review 😄

@calebporzio
Copy link
Collaborator

Thanks @HugoDF, but going to hold off on adding "object" syntax for the style attribute for now.

May remove the object syntax from the class attributes in v3 - undecided yet. I will put a note in the Roadmap and reference this PR.

Thanks for the contribution!

@OwenMelbz
Copy link

@calebporzio would you be able to clear up the thoughts around removing object support from the :class ? it seems a massively common method to conditionally apply classes?

<input :class="{'input-valid': isValid, 'input-invalid': !isValid && isDirty}" />

@HugoDF
Copy link
Contributor Author

HugoDF commented Apr 27, 2020

@OwenMelbz the rationale is two fold, first is that it's achievable in userland with a util function that converts objects into an array (or string), second is that it's not implemented exactly in the same way as Vue and that has caused issues.

@OwenMelbz
Copy link

Hey @HugoDF I appreciate the "issues caused" would add a complication.

So would that mean that if anybody wanted to use the syntax above they would need to create a global function that needs access to the data? Wonder if you could mockup what you mean for a real-world app, would it be something like...?

<div x-data="{value: '', isValid: false, isDirty: false}">
    <input :class="mergeArrays(inputClasses($data), otherClassLogic())" />
</div>
window.mergeArrays = () => {
    let finalArray = [];

    arguments.forEach(arg => {
        finalArray = finalArray.concat(arg);
    });

    return finalArray;
}

window.otherClassLogic = () => {
    // blah do ajax lookup or something unique to this field
    return ['input-account-already-exists'];
}

window.inputClasses = ({isValid, isDirty}) => {
    let classes = [];

    if (isValid) {
        classes.push('input-valid')
    }

    if (!isValid && isDirty) {
        classes.push('input-invalid');
    }

    return classes;
}

Thanks!

@HugoDF
Copy link
Contributor Author

HugoDF commented Apr 27, 2020

Hmm no it would have a similar API

:class="objClass({ 'valid': valid, 'invalid': !valid })"

Implementation would then be Alpine agnostic.

@philwolstenholme
Copy link
Contributor

@OwenMelbz one option might be to use the Classnames package to compose the list of class strings. I've put together a small demo here: https://jsfiddle.net/philw_/ydvm8r1h/

Btw I found this issue while looking at the Roadmap, I've never used Alpine before but have been experimenting with adapting the dropdown from the README and have really enjoyed it!

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.

Inline Styles
5 participants