Fix `project_messages` alert/checkbox HTML in obs form

MushroomObserver / mushroom-observer

A website for sharing observations of mushrooms.

https://mushroomobserver.org

MIT License

77 stars 26 forks source link

Fix `project_messages` alert/checkbox HTML in obs form #2175

Closed nimmolo closed 3 weeks ago

nimmolo commented 3 weeks ago

Sorry - I should have noticed this on the original PR. This doesn't change much, it just moves some things around (invisibly).

Eliminates duplicate DOM id observation_projects, which is invalid HTML
Moves flash_warning from the view to the controller, so flash messages are available to all rendering methods, in the case the obs form gets Turbo-ized. (Generally, flash needs to happen in the controller if we ever want to flash inside a "modal" form or use something like ViewComponent, but I think it's also just better practice. I'm aiming to gradually move all flash calls to the respective controllers when possible.)
On the form, moves the whole message and checkbox into the #observation_projects div, and move the checkbox inside the .alert div, as it is for form-name-feedback et. al.

coveralls commented 3 weeks ago

coverage: 94.454%. remained the same when pulling b50cdadc4ed0d60a835c7ae58b8065d463b07099 on nimmo-fix-project-messages into 38b163f25e902ab821d7e49b2e20c148720ec850 on main.

coveralls commented 3 weeks ago

coverage: 94.454%. remained the same when pulling b50cdadc4ed0d60a835c7ae58b8065d463b07099 on nimmo-fix-project-messages into 38b163f25e902ab821d7e49b2e20c148720ec850 on main.

nimmolo commented 3 weeks ago

Thanks @JoeCohen - will merge then.

nimmolo commented 3 weeks ago

Oops - I'm on the train and cannot SSH to deploy on production. If @JoeCohen or @mo-nathan can easily deploy, please do; otherwise i'll do it later tonight.

Just wanted to give a heads up, because we've had bad experiences with my merged but undeployed code before!

JoeCohen commented 3 weeks ago

I'll deploy it now.

On Mon, Jun 10, 2024 at 5:30 PM andrew nimmo @.***> wrote:

Oops - I'm on the train and cannot SSH to deploy on production. If @JoeCohen https://github.com/JoeCohen or @mo-nathan https://github.com/mo-nathan can easily deploy, please do; otherwise i'll do it later tonight.

Just wanted to give a heads up, because we've had bad experiences with my merged but undeployed code before!

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2175#issuecomment-2159549386, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFCYLFB6IZHDOSG54ILZGZAJ5AVCNFSM6AAAAABJDF6FR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJZGU2DSMZYGY . You are receiving this because you were mentioned.Message ID: @.***>

JoeCohen commented 3 weeks ago

Deployed.

On Mon, Jun 10, 2024 at 5:31 PM Joseph Cohen @.***> wrote:

I'll deploy it now.

On Mon, Jun 10, 2024 at 5:30 PM andrew nimmo @.***> wrote:

Oops - I'm on the train and cannot SSH to deploy on production. If @JoeCohen https://github.com/JoeCohen or @mo-nathan https://github.com/mo-nathan can easily deploy, please do; otherwise i'll do it later tonight.

Just wanted to give a heads up, because we've had bad experiences with my merged but undeployed code before!

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2175#issuecomment-2159549386, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFCYLFB6IZHDOSG54ILZGZAJ5AVCNFSM6AAAAABJDF6FR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJZGU2DSMZYGY . You are receiving this because you were mentioned.Message ID: @.***>

nimmolo commented 3 weeks ago

Thanks @JoeCohen