-
Notifications
You must be signed in to change notification settings - Fork 6
Create topics #44
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
Create topics #44
Conversation
7e01f0b to
a3e1e8b
Compare
8d418e7 to
736da1d
Compare
736da1d to
5a5a9da
Compare
|
|
||
| <div class="mt-4"> | ||
| <div class="col-12 d-flex justify-content-end"> | ||
| <%= form.submit "Create Topic", class: "btn btn-primary me-1 mb-1" %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is also used for the #edit action, maybe changing the wording to "Save" would be better to apply both for #create and #edit.
| <%= form.submit "Create Topic", class: "btn btn-primary me-1 mb-1" %> | |
| <%= form.submit "Save Topic", class: "btn btn-primary me-1 mb-1" %> |
| <div class="col-md-4"> | ||
| <div class="form-group"> | ||
| <%= form.label :title, style: "display: block" %> | ||
| <%= form.text_field :title, id: "basicInput", class: "form-control", autofocus: true %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we have a few "id: "basicInput"" in the app, sometimes within the same page, but I don't think we actually need them? I think we got them when we copy/pasted code from Mazer and forgot to remove it.
| t.text :description | ||
| t.references :language, null: false, foreign_key: true | ||
| t.references :provider, null: false, foreign_key: true | ||
| t.boolean :archived, default: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the wiki, Danny had mentioned using enumerables for states like "active" and "archived". Do we want to do this or should we go with a boolean?
|
Sorry, I hadn't realised we already had another PR for this, so please ignore my comments! |
Resolves #18