Skip to content
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

12363 update paragraph spacing on journal table #12435

Merged
merged 7 commits into from
May 4, 2023

Conversation

arthanson
Copy link
Collaborator

@arthanson arthanson commented May 2, 2023

Fixes: #12363

Updates paragraph spacing on journal table so comments are shown correctly spaced. Only effects journal-table as other tables should display with condensed paragraphs.

@arthanson
Copy link
Collaborator Author

changed to a generic spaced-paragraph-table utility class. let me know if you want any further changes.

@jeremystretch
Copy link
Member

Rather than adding a one-off class for this, let's do some digging to determine why and how the margin is being removed in the first place. We can attribute the style to this rule in netbox.scss:

p {
// Remove spacing from paragraph elements within tables.
margin-bottom: 0;
}

Running git blame on this file show that the change was part of a large array of changes two years ago, likely when the v3.0 UI was still being refined. So I think it's reasonable to just remove it entirely. If complaints arise that this results in suboptimal rendering elsewhere in the UI, we can then add a "condensed" class to remove the margin where needed.

@jeremystretch
Copy link
Member

I actually remember now why we added this. Rendered Markdown text, even a single line, always creates a <p> element. This adds unnecessary white space to the bottom of the table cell (which really triggers some people).

I wonder if we can easily apply the rule only to the last <p> element in the cell. This would retain white space between paragraphs without any extraneous space at the end.

@kkthxbye-code
Copy link
Contributor

wonder if we can easily apply the rule only to the last <p> element in the cell.

I'm not really a CSS aficionado, but the :last-child pseudo-class would probably work.

@arthanson
Copy link
Collaborator Author

That historical context helps. Reverted and made the last-child to 0 and the regular p table spacing to .5em - this allows you to still see paragraph spacing, but gets rid of it at the bottom and reduces the spacing to conserve line height:
Monosnap Devices | NetBox 2023-05-03 08-21-41

@jeremystretch
Copy link
Member

Thanks @arthanson!

@jeremystretch jeremystretch merged commit 683ef30 into develop May 4, 2023
@jeremystretch jeremystretch deleted the 12363-journal-paragraph-render branch May 4, 2023 18:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New paragraph line breaks do not render in journal entry table comments
3 participants