Divider with note is overlapping

[dstoyanoff]

Preflight Checklist

• I have completed all Troubleshooting Steps.
• I'm on the latest version of Directus.
• There's no other issue that already describes my problem.

Describe the Bug

When setting up a divider in a collection and adding a Note for it, the note does not add an offset from the divider itself, making it sticked to the line, which isn't consistent with the rest of the UI.

To Reproduce

Create a presentation field of type Divider and add a note. Go to the collection and observe the result

What version of Directus are you using?

rc.95

What version of Node.js are you using?

What database are you using?

What browser are you using?

Chrome

What operating system are you using?

How are you deploying Directus?

[azrikahar]

Seems like this is caused by the negative margin bottom here:

directus/app/src/interfaces/presentation-divider/presentation-divider.vue

Lines 46 to 49 in cb7b6a9

	.v-divider {
	margin-top: 10px;
	margin-bottom: -10px;
	}

which was added in #7972. @benhaynes I believe this is added to make the gap between presentation divider and input smaller right? Would like to get your thoughts on this before I do any abrupt change here.

[benhaynes]

That's correct! The divider was evenly spaced between the fields above and below. These new margins helped keep the divider closer to the section it was labeling... But unfortunately also his making the note too close.

Any ideas? 🤔

[licitdev]

@benhaynes Would this be okay? Removed the negative margin-bottom.

Also, the margin-top: 10px in the screenshots below was removed.
Is the gap with the above element enough?

These new margins helped keep the divider closer to the section it was labeling

Lastly, which section close to labeling did you mean? Any example?

Title + Note

Title + Note + Increase Margin Top

Title

Title + Increase Margin Top

Note

Note + Increase Margin Top

Empty

Empty + Increase Margin Top

[benhaynes]

@licitdev — this is super helpful, thank you!! These options offer good customization, my main concern was that the normal/default state places the divider equally between form sections:

In this example, the divider is the same distance to the green button (above) as to the gray button (below). My design mind wants the divider always closer to the form section it is labeling (the part with the gray button). I know the "Margin Top" adds this, but I think that should be the default. @rijkvanzanten ?

Also, I decreased the form-gap-vertical a week ago, and that is helping here.

[rijkvanzanten]

Yeah i'm down to change the default value of that margin option to true, or just have it hardcoded enabled at all time (not sure why that needs to be configurable)

[licitdev]

Yeah i'm down to change the default value of that margin option to true, or just have it hardcoded enabled at all time (not sure why that needs to be configurable)

@rijkvanzanten The add top margin already defaults to true in PR #8379.
@benhaynes The gap is reduced due to issue #8374.

[benhaynes]

I think the only reason to make the margin-top configurable is in case it's at the top of the form (so it doesn't just push everything down for no reason). If that is resolved somehow, I would love to ditch this option and just hard-code it!

[rijkvanzanten]

That should be doable with some css selection magic 👍🏻

[licitdev]

I think the only reason to make the margin-top configurable is in case it's at the top of the form (so it doesn't just push everything down for no reason). If that is resolved somehow, I would love to ditch this option and just hard-code it!

Yup that margin-top can be toggled now. We only have to remove margin-bottom: -10px;.

@benhaynes I would like to ask if you would still like to have the margin-top: 10px?
Here's a sample without it removed.

I had removed it to make it exactly vertically middle for the Empty type...
If this is okay, the fix will be simply to remove the entire code block below.

directus/app/src/interfaces/presentation-divider/presentation-divider.vue

Lines 46 to 49 in cb7b6a9

	.v-divider {
	margin-top: 10px;
	margin-bottom: -10px;
	}

[rijkvanzanten]

@licitdev @benhaynes we can't rely on negative margins for things like these, because it will always result in issues when used in a different context (as proven by this issue's existence) 👍🏻

[licitdev]

@rijkvanzanten Yup definitely. I just let the existing negative margin remain there as I don't wish to break the uniformity of UI. 😂👍🏻

[benhaynes]

OK, cool. So let's hardcode the interface to ALWAYS have the top margin, and remove the option altogether. 👍

[licitdev]

@benhaynes Do you mean to remove the Increase Margin Top option as shown in below screenshot?

The below screenshot shows the default options, whereby the additional margin top is set to TRUE.

The option to toggle is very helpful particularly if the divider contains no title and is just used as a line separator.
Hence if it is ALWAYS having the top margin except when it is the first divider, the divider becomes less flexible in my opinion... 🤔

[benhaynes]

Hmm, good point. What if we ALWAYS had the top margin when there is a Title, but NEVER have it if there is no title?

Also, not sure if I like the styling of the "Inline Title" option.

[licitdev]

Hmm, good point. What if we ALWAYS had the top margin when there is a Title, but NEVER have it if there is no title?

Yes that should be possible, the divider will also need to have the top margin removed if it happens to be the first field.
But how should the interface be?
It would mean that we would need to have conditions working, disabling the Increase Margin Top toggle and also setting it to be true in the interface...

I have tried to mess around with the CSS such as :first-child and parent selector etc, but couldn't get it working...
@rijkvanzanten Will require your guidance on css selection magic for scoped CSS. 🤓

[rijkvanzanten]

I think we should get rid of the "Margin Top" option completely, in favor of just doing it correctly by default 👍🏻

[rijkvanzanten]

I have tried to mess around with the CSS such as :first-child and parent selector etc, but couldn't get it working...
@rijkvanzanten Will require your guidance on css selection magic for scoped CSS. 🤓

I can pick this up 🙂 Do you wanna prepare a PR and I'll add this in on top of that?

[licitdev]

I think we should get rid of the "Margin Top" option completely, in favor of just doing it correctly by default 👍🏻

Would that mean that users will only have the latter option with a much bigger gap?

Empty

Empty + Increase Margin Top

[rijkvanzanten]

I think that's where Ben's suggestion comes in: Only add the top margin when a Title has been configured 🙂

[licitdev]

Ah... Means if without a title, it'll be without the margin-top. Crystal clear now.

I can pick this up 🙂 Do you wanna prepare a PR and I'll add this in on top of that?

Yup, working on it~