joy-framework / joy

A full stack web framework written in janet
https://joy.swlkr.com
MIT License
535 stars 30 forks source link

Refactor form-helper functions into more streamlined API #106

Open Khachig opened 1 year ago

Khachig commented 1 year ago

Simplifying the function APIs

I've refactored the form-helper functions into a simpler — in my opinion — API to avoid passing what I thought was a needless dictionary and key to the functions. I think this refactor removes some redundancy from the API.

Currently

Most of the time, I want to create a text input form with the name and placeholder attributes, along with some classes. In order to do this currently, I would have to pass in the required struct but keep it empty since I don't want to set a default value attribute.

(text-field {} :textfield :placeholder "Enter text" :class "my-class")
# => <input type="text" name="textfield" placeholder="Enter text" class="my-class" />

If I do want to add a default value, I have two ways to do it:

  1. Using the struct (thereby also duplicating the key)
    (text-field {:textfield "Default value"} :textfield :placeholder "Enter text" :class "my-class")
    # => <input type="text" name="textfield" value="Default value" placeholder="Enter text" class="my-class" />
  2. Using the default HTML value attribute (still have to pass an empty struct)
    (text-field {} :textfield :value "Default value" :placeholder "Enter text" :class "my-class")
    # => <input type="text" name="textfield" value="Default value" placeholder="Enter text" class="my-class" />

    I think we can do away with this redundancy since there's already a default way to define the default value of HTML elements. Another issue is that the val dictionary being passed into the field function is only used to retrieve the value of the field with the same key in the second argument, which makes any other fields in the dictionary pointless to the construction of the input element.

Whatever slight benefit that could be gained by reusing a table of attributes for the sole purpose of extracting the value attribute is outweighed by the redundant API. Allowing the first method also opens up the possibility for inconsistency in the codebase if the same thing is done differently in different places. Pre-filling input fields is also not the desired default behavior in most cases for non-hidden inputs so an empty struct would have to be passed in most of the time.

After this refactor

By default, attributes other than name are not set unless done explicitly. There is also no need to pass a dictionary and duplicate the key but simply a keyword or string for the name attribute and the optional attributes after that. I believe this makes for a much simpler and intuitive API. Here's the code to do the same thing as above.

(text-field :textfield :placeholder "Enter text" :class "my-class")
# => <input type="text" name="textfield" placeholder="Enter text" class="my-class" />
swlkr commented 1 year ago

Oh I thought I replied to this.

This is a good change, I guess my only hang up is that if anyone is out there using this, when they upgrade it's going to break them.

Is there a way we can make this backwards compatible?

Maybe check the number of arguments in the function or come up with new names text-input instead of text-field or my last idea would be to come up with a new module form-helper2 or something and people can opt in to the better api?

Khachig commented 1 year ago

Apologies for disappearing. Life gets in the way sometimes. Just wanted to say that I agree with your comment and I'll make an update soon.