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

add selectFieldTag helper #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dmanto
Copy link
Contributor

@dmanto dmanto commented Jan 23, 2024

Proposal to implement the selectFieldTag helper.

The implementation aims to keep some consistency between mojo.pl and mojo.js templates for the select_field helper.

Key Points:

  • Main signature: selectFieldTag(name, [array of options], attributes)
  • array of options: can be strings, strings followed by attributes (will apply to each option), or arrays (see below)
  • optgroup: While mojo.pl utilizes a Mojo::Collection for , I've opted for nested arrays inside the array of options.
  • optgroup format: First element of array must be a string, that would convert to corresponding label optgroup attribute. After that, element are treated as normal option items. After the array you can include an object with optgroup desired attributes.
  • Test cases: So far they are just mojo.pl cases translated.

Test cases for mojo.pl translations are as follows:

  1. mojo.pl base case:
%= select_field a => ['b', c(c => ['<d', [ E => 'e'], 'f']), 'g']

... translates to mojo.js:

%= await tags.selectField("a", ["b", ["c", "<d",  "E", {value: "e"}, "f"], "g"])
  1. mojo.pl case with multiple attribute:
%= select_field foo => [qw(bar baz)], multiple => 'multiple'

... translates to:

%= await tags.selectField("foo", ["bar", "baz"], {multiple: "multiple"})
  1. mojo.pl case with disabled attribute:
%= select_field bar => [['D' => 'd', disabled => 'disabled'], 'baz']

translates to:

%= await tags.selectField("bar", ['D', {value:  'd', disabled: true}, 'baz'])
  1. mojo.pl case with : (uses Mojo::Collection for group)
%= select_field yada => [c(test => [qw(a b)], class => 'x')];

...translates to: (uses a nested array for group)

%= await tags.selectField ("yada", [["test", "a", "b"], {class: 'x'}])
  1. mojo.pl case with preselected options:
%= select_field h => [['I' => 'i', selected => undef], ['J' => 'j']]

...translates to:

%= await tags.selectField("h", ['I', {value: 'i', selected: true}, 'J', {value: 'j'}])

@kraih
Copy link
Member

kraih commented Jan 23, 2024

Looks like test coverage still needs a little work.

@kraih
Copy link
Member

kraih commented Jan 23, 2024

Even in Perl i've never actually used the <select> helper, the arguments are just too complex to remember. Is it really worth adding it to mojo.js in this super complex form as well?

@kraih
Copy link
Member

kraih commented Jan 24, 2024

Perhaps this would be better off in a plugin. Would also make documenting the complex usage more straight forward. 🤔

@dmanto
Copy link
Contributor Author

dmanto commented Jan 25, 2024

Even in Perl i've never actually used the <select> helper, the arguments are just too complex to remember. Is it really worth adding it to mojo.js in this super complex form as well?

About the complexity, may I suggest to implement it in a very different way, more like a tags.formFor() signature?.

That would be something like:

%= await tags.selectFieldTag("some_name", {...attrs}, "select content")

Despite being very different from Perl implementation, I think it would be much easier to understand and document. Implementation should be very easy too, just looking at formFor code it should be just one line. It would benefit from the existence of Template Blocks to generate the content inside <select></select> tag

Also an <option> tag helper should be added, but it seems straightforward also.

Then there is the issue about pre-populating the selected option(s) according to current data when rendering an edit form, but this is something probably needs more discussion.

@kraih
Copy link
Member

kraih commented Jan 31, 2024

Separating <select> and option into two helpers kinda defeats the purpose.

@dmanto
Copy link
Contributor Author

dmanto commented Feb 1, 2024

Separating <select> and option into two helpers kinda defeats the purpose.

mmm not sure about that. I see it like the <form> tag with its <input> elements

@dmanto
Copy link
Contributor Author

dmanto commented Feb 1, 2024

About the purpose of using tags, for me one reason to use <form> related tags would be to simplify the rendering of a form considering existing data in the stash.

Consider as a reference the pg.js blog example. If, for instance, we want to add a priority field to a post, with possible values High, Normal and Low we would start modifying the views/posts/_form.html.tmpl to something like: (untested)

<{formBlock}>
  <label for="title">Title</label>
  <br>
  %= await tags.textField('title', {value: post.title});
  <br>

         %# ADDED 

  <label for="priority">Priority: </label>
  <br>
  <select name="priority" id="priority">
    <option value="">--Please select priority of the post--</option>
    <option value="high">High</option>
    <option value="normal">Normal</option>
    <option value="low">Low</option>
  </select>
  <br>

         %# END ADDED 

  <label for="body">Body</label>
  <br>
  %= await tags.textArea('body', {}, post.body)
  <br>
  %= await tags.submitButton(caption)
<{/formBlock}>
%= await tags.formFor(target, {class: 'blog-form'}, await formBlock())

But this has a problem because the existing value for the priority field, despite it is probably loaded into post.priority by the controller, will not be reflected in the rendering of the select block.

What we have to do is to add some template coding to include a "selected" attribute on the option with the value attribute matching the post.priority value.

So it would be nice if the option tag somehow helps us on this issue.

Also probably something could be done with the other input tags, as passing the post object directly to the form so all inputs can reflect its value corresponding to its name attribute.

@kraih
Copy link
Member

kraih commented Feb 4, 2024

mmm not sure about that. I see it like the <form> tag with its <input> elements

The <form> helper is different though, because of the urlFor integration.

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.

2 participants