phoenixframework / phoenix

Peace of mind from prototype to production
https://www.phoenixframework.org
MIT License
21.12k stars 2.85k forks source link

Remove margin top on the input component if no label is provided in `core_components.ex` #5832

Open NTurchi opened 2 months ago

NTurchi commented 2 months ago

Environment

Elixir 1.16.1 (compiled with Erlang/OTP 25)

* Phoenix version (mix deps): `1.7.12`
* Operating system: MacOS

### Actual behavior

Hello Phoenix Team πŸ‘‹ 
This is a minor issue with the input component from the `core_components.ex` file (used in `phx.gen.live`)
A margin top is added by default for the spacing between the label and the input. 

https://github.com/phoenixframework/phoenix/blob/69685f783bc60d2a124fc2fb48028b065bc24b22/priv/templates/phx.gen.live/core_components.ex#L357-L361

However it is not removed if a label is not provided. Which causes alignment issue when combining the input component with other elements on our UI. 

![mt-2-top-screen](https://github.com/phoenixframework/phoenix/assets/16674693/ed0bdfea-5f1d-4843-a3d6-c25ea797168a)

Of course this can be fixed easily by modifying the `core_component` in my project but I guess this can be a quick win improvement to update the `core_components.ex` directly in Phoenix. I can propose a PR if you think it's worth making a small update πŸ™ 

<!--
Describe the actual behaviour. If you are seeing an error, include the full message and stacktrace.

If possible, also provide a single file app that reproduces the behaviour, you can start here:
https://github.com/wojtekmach/mix_install_examples/blob/main/phoenix.exs
-->

### Expected behavior

Avoid adding a margin to the input if a `label` is not provided to the input component.

Fix: one idea could be to put the `mt-2` class behind a condition, e.g: 

```elixir
class={[ 
    "block w-full rounded-lg text-zinc-900 focus:ring-0 sm:text-sm sm:leading-6 min-h-[6rem]", 
    @errors == [] && "border-zinc-300 focus:border-zinc-400", 
    @errors != [] && "border-rose-400 focus:border-rose-400",
    # new condition
    label != nil && "mt-2",
]} 
josevalim commented 1 month ago

Maybe we should add mb-2 to the label instead? A PR is welcome.