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

Simplify prop binding and conditional props #109

Closed
krmax44 opened this issue Dec 28, 2021 · 2 comments
Closed

Simplify prop binding and conditional props #109

krmax44 opened this issue Dec 28, 2021 · 2 comments
Assignees
Labels
Semver: Next Will make it's way to the bleeding edge version of the package Type: Enhancement Improving an existing feature

Comments

@krmax44
Copy link

krmax44 commented Dec 28, 2021

Description

Currently, it can be quite a chore passing props to elements. For example, when creating an input field component:

<input type="text" name="username" placeholder="Username" class="bg-white rounded ...">

Now, let's make this more dynamic. I'd like to set the placeholder, name and optionally value attributes and overwrite classes if needed. The type should be text by default, but also be customizable. Furthermore, optionally an error can be provided. In a template, it would be used something like this:

@!component('forms/input', {
  placeholder: 'Your name',
  name: 'name',
  class: 'w-full',
  error: 'Please fill out this field'
})

Implementing this is quite difficult, especially when compared to other templating languages:

<input
  type="{{ type ?? 'text' }}"
  name="{{ name }}"
  placeholder="{{ placeholder }}"
  class="mt-2 rounded border-gray-300 shadow-sm focus:border-blue-300 focus:ring focus:ring-blue-500 focus:ring-opacity-50 {{ error ? 'ring ring-red' : '' }} {{ classes ? classes : '' }}"
  {{ $props.serializeOnly(['value']) }}
  {{{ error ? `aria-errormessage="${e(error)}"` : '' }}} <!-- this looks messy and fragile to me -->
/>

I have found some issues with this:

  • The class attribute will contain whitespaces at the end. It's possible for classes to be printed twice, if predefined classes are also passed in prop. Also, using the conditional operator adds a lot of boilerplate code. Just feels a bit strange to me.
  • $props.serializeOnly seems handy at first, but will not work in many cases, as it returns the string undefined, when passed that value. Furthermore, it doesn't work when the attribute key does not match the prop key, as with aria-errormessage/error.
  • In general, I find myself often limited by the template's capabilities, which is a shame for Adonis users, as it seems to be the only option there so far. Or maybe I just haven't found the right helpers yet.

Proposed changes

Firstly, I'd find it much more intuitive if undefined would render to an empty string, instead of the string literal undefined. Vue and React (JSX) behave the same in this, and I'd assume most other templating languages behave the same.

Also, I'd love if binding attributes would be easier. Coming from Vue, I'm used to the comfort of bindings like these:

<input
  :type="type ?? 'text'" <!-- default can also be provided in component definition -->
  class="mt-2 rounded border-gray-300 shadow-sm focus:border-blue-300 focus:ring focus:ring-blue-500 focus:ring-opacity-50"
  :class="[{ 'ring ring-red': error }, classes]"
  :value="value"
  ...
/>

Maybe Edge could take some inspiration from other modern templating languages, like Vue or JSX.

Thanks for your time and great work!

@stale
Copy link

stale bot commented Apr 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Abandoned Dropped and not into consideration label Apr 8, 2022
@stale stale bot closed this as completed Apr 15, 2022
@thetutlage thetutlage reopened this Aug 7, 2023
@stale stale bot removed the Status: Abandoned Dropped and not into consideration label Aug 7, 2023
@thetutlage thetutlage self-assigned this Aug 14, 2023
@thetutlage thetutlage added Type: Enhancement Improving an existing feature Semver: Next Will make it's way to the bleeding edge version of the package labels Aug 14, 2023
@thetutlage
Copy link
Member

We released a major version of Edge recently and improved the Props API a lot. Please lemme know if that works for you. https://edgejs.dev/docs/components/props

Feel free to re-open this issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Semver: Next Will make it's way to the bleeding edge version of the package Type: Enhancement Improving an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants