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

the selected attribute on the option tag is not set #66

Closed
sebastjan-hribar opened this issue Jun 7, 2016 · 6 comments
Closed

the selected attribute on the option tag is not set #66

sebastjan-hribar opened this issue Jun 7, 2016 · 6 comments
Assignees
Milestone

Comments

@sebastjan-hribar
Copy link
Contributor

Hi,

I'm using select helper and the selected attribute on the option tag is not automatically set. For example, when displaying an entity for edit and update.

I've setup a small bookshelf with book and author entities. It's minimalistic just to get the minimum required environment ready.

Hanami data:
hanami, '0.7.2'
hanami-model, '> 0.5'
hanami-helpers (
> 0.3)

Sample app: https://github.com/sebastjan-hribar/hanami-bookshelf

This is the html from inspecting page:

<h2>Edit book</h2>

<form action="/books/1" method="POST" accept-charset="utf-8" id="book-form">
<input type="hidden" name="_method" value="PATCH">
<div class="input">
<label for="book-title">Title</label>
<input type="text" name="book[title]" id="book-title" value="Test Book">
</div>
<div class="input">
<label for="book-author">Author</label>
<select name="book[author]" id="book-author">
<option value="1">Best Author</option>
<option value="2">Second Best Author</option>
</select>
</div>
<div class="controls">
<button type="submit">Update Book</button>
</div>
</form>

I've similar problems in my other app, I've setup this one just for this select problem.

Please let me know if I should add further information.

regards,
Sebastjan

@jodosha
Copy link
Member

jodosha commented Jun 7, 2016

@sebastjan-hribar Thanks for reporting this problem.

I've cloned your demo app (thanks for that 🙏 ). It can't work, because the framework only selects an <option> tag in case of a HTTP param has a value that applies. This is useful in case of failing form submission: you want to render the form again, but won't loose the input that the user typed.

We should expand select with a new option to cover your case.

select :author, authors, selected: book.id

Then this check should be expanded accordingly:

WDYT? Is it something that you want to implement? Thanks!

@sebastjan-hribar
Copy link
Contributor Author

@jodosha I'm sorry for my late reply.

Thank you for the feedback.

select :author, authors, selected: book.id is something I had in mind for this scenario.

I'd be interested in trying to implement this, however I'm not confident I can do as good a job and as fast as an experienced person. But I have to start somewhere and sometime, right? 😄
Let me know if you would like to entrust this to someone else, or I should have a go at it.

On a second note:
I've pushed the updated test app to master where I've added a spec for the prompt option. The spec fails and also if I visit the page manually, the prompt is not there.

Before opening another issue, could you have a look at the template
and the spec to see I'm not making mistakes in my code.

regards,
Sebastjan

@jodosha
Copy link
Member

jodosha commented Jun 8, 2016

@sebastjan-hribar I trust you.

There are plenty of tests here:

describe "#select" do
, just add another one to cover this case and make it to pass. 😉


The :prompt option was introduced on Mar 10, but not released yet. In your project, you're using a stable version of hanami-helpers, that's why you ain't no support for it yet.

@sebastjan-hribar
Copy link
Contributor Author

@jodosha can you look at this gist? After forking and bundle install I ran rake test and get the error in the gist.
Also this one is strange: L25

@jodosha
Copy link
Member

jodosha commented Jun 20, 2016

@sebastjan-hribar master branch is now clean (see b534567), please go ahead with this fix. Sorry about the trouble.

@jodosha
Copy link
Member

jodosha commented Jul 5, 2016

Implemented by #69

@jodosha jodosha closed this as completed Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants