Fix code action icon position in top heading

[hichemfantar]

Eliminate unnecessary top padding from the action icon to improve alignment in the top heading.

before

after

[github-actions]

📦 Docs artifacts are ready: https://github.com/elixir-lang/ex_doc/actions/runs/12888458292/artifacts/2461723578

[josevalim]

The alignment is necessary for full page render, otherwise it feels very out of place:

Can we make it work on both cases?

[josevalim]

Perhaps as a flex element with some tiny gap between them and vertically aligned in the center?

[hichemfantar]

fixed 🚀

[josevalim]

Apologies but I still see undesired behaviour in your version:

Curiously, it works well on the latest release, so this was a recent regression: https://hexdocs.pm/ex_doc/ - so we should perhaps revert to the behaviour there, which is make it float on top right consistently. This seems to be best behaviour of the ones I tested. WDYT?

[hichemfantar]

i feel the new behavior is better because it gives titles more space to work with and bringing the code link lower in the screen makes it easier to mobile users to reach it

old

new

[hichemfantar]

another example

old

new

[hichemfantar]

We also no longer have to worry about perfectly aligning the icon with the title

[hichemfantar]

also looks good when the title isn't long and works better because aligning with flex is more consistent instead of padding

[hichemfantar]

whoever made the change to flex had the right idea but forgot to do some more testing which is addressed in this pr

[josevalim]

I prefer the currently published version, so let's wait to see if @DavidOliver has a preference.

PS: I deleted yours and mine comment about the alignment, to keep this one on topic and easier for others to review.

[DavidOliver]

If I remember correctly, I switched to Flexbox as part of a commit that dealt with a screenreader issue.

Can we put the action icon(s) below the heading at narrow viewport widths (wrap, both elements full width) and to the right of the heading at wider viewports (nowrap, fixed width for action icon area)? I think that would satisfy both concerns mentioned above?

Thanks.

[hichemfantar]

@DavidOliver i think my fixes do just that if I’m not wrong

[DavidOliver]

In this comment, the new screenshot seems to show the action icon below the heading. For wider viewports, I think it makes sense to keep the action icon where it was.

At narrower (mobile portrait) viewports, I can see that it may make sense to allow the heading to take the full width available, with the action icon being underneath.

[hichemfantar]

@DavidOliver that only happens when the title is too long then the icon gracefully goes under it

[DavidOliver]

I realise that, and I'm suggesting that it would be better for the icon to stay where it was and that the heading text wraps within its flex item.

Narrow viewports (maximal width for heading text):

Heading text that is
really rather long
and very descriptive
                [icon]

Medium/wide viewports (more visually pleasing alignment?):

Heading text that is really rather long and  [icon]
very descriptive

This would change the flexbox wrap/nowrap at a breakpoint.

However, I think @josevalim may not like the action icon being under the heading text at narrow viewports even if it does allow more horizontal space for the heading text, in which case we can find another way of fixing the issue in your original screenshot. I could do this if you like.

[hichemfantar]

I guess enabling flex wrap only on mobile via a breakpoint is a good solution.

[hichemfantar]

sure thing you can implement it if you wish

[DavidOliver]

Okay. Let's wait to hear back from José and go from there.

[josevalim]

Let's try the one @DavidOliver suggested then, because I currently prefer the one on the top always, but maybe I will be happier with the one on the middle always :D

[hichemfantar]

alright so the consensus is on this

[josevalim]

💚 💙 💜 💛 ❤️

[josevalim]

If the browser thinks that's "center", who am I to disagree :D

[hichemfantar]

are you talking center vertically or horizontally?

[josevalim]

Vertically Anyway, the code is good, I am just surprised by the browser :)

[DavidOliver]

@josevalim, if I understand correctly, this PR doesn't yet reflect @hichemfantar's latest screenshots/mockup. I don't think the change in wrap at a certain breakpoint has been done yet?

Also, I think that if align-items is set to center, the action icon will be centred even for headings that run over multiple lines, which will look strange. (Though I haven't tried any of this out yet.) I had align-items start with padding so that the icon was aligned with the first line.

[josevalim]

Oh, ok. I see. Apologies for merging too fast @hichemfantar. A new PR is appreciated. :)