Improve line-height CSS

[seancolsen]

@pavish why have we had line-height: 1em; set globally?

That CSS has some weird effects, including making underscores nearly invisible in tab names at certain zoom settings.

Adjusting the line-height is the best way to fix that issue I think, and to me it makes the most sense to unset it globally (as this PR does).

Before	After

I imagine we'll want to tweak a bunch of the typography stuff once we do our overall styling. If we do want to customizes the line height, MDN recommends always using unitless values for line-height.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the master branch of the repository
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[codecov-commenter]

Codecov Report

Merging #999 (2800517) into master (3ebc91e) will not change coverage.
The diff coverage is n/a.

❗ Current head 2800517 differs from pull request most recent head 40e9b2a. Consider uploading reports for the commit 40e9b2a to get more accurate results

@@           Coverage Diff           @@
##           master     #999   +/-   ##
=======================================
  Coverage   92.46%   92.46%           
=======================================
  Files          89       89           
  Lines        3200     3200           
=======================================
  Hits         2959     2959           
  Misses        241      241           

Flag	Coverage Δ
pytest-backend	92.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ebc91e...40e9b2a. Read the comment docs.

[kgodey]

Perhaps we should add mathesar/templates/ to the frontend list in our CODEOWNERS file?

[pavish]

@kgodey

Perhaps we should add mathesar/templates/ to the frontend list in our CODEOWNERS file?

Yes, we should. We could also include mathesar/urls.py and mathesar/views.py since they also deal with the frontend urls and preloading.

[kgodey]

Yes, we should. We could also include mathesar/urls.py and mathesar/views.py since they also deal with the frontend urls and preloading.

Those should be @centerofci/mathesar-maintainers since they're also Python code, backend should look at them too. Can you go ahead and make the update?

[pavish]

@seancolsen

Why have we had line-height: 1em; set globally?

Line height with no default varies oddly across browsers, leading to slight misalignment of elements in browsers we do not test on.

Setting a proper line height also depends on the font we use and we only use system fonts in the moment, which also makes it differ across operating systems.

Setting a line-height globally avoids this issue. We should be setting individual line heights to parts of code where we encounter issues as you've posted above.

As of now, removing line-height has effects in other parts of the page, leading to misalignment of icons and extra leading heights for inputs.

We could change 1em to 1 instead.

We can come back to this when we do our overall styling.

[pavish]

@kgodey

Those should be @centerofci/mathesar-maintainers since they're also Python code, backend should look at them too. Can you go ahead and make the update?

Raised #1000 for this.

[seancolsen]

@pavish

Line height with no default varies oddly across browsers

Cool, let's set a default then.

But a default of 1 seems too narrow. Can we use 1.2 instead (as in the most recent commit I pushed to this PR)? That looks better, is more consistent with browser defaults, and fixes the UX issue with overflow of the underscore character in tabs.

From MDN:

Desktop browsers (including Firefox) use a default value of roughly 1.2

[pavish]

@seancolsen The changes look fine.

The issue with finding the correct line-height is that fonts differ on the baseline to descender distance, as well as the cap-height distance.

Currently we only use system fonts, so our font will differ based on the OS and this will also lead to subtle visual differences in positions. Usually, that wouldn't be of concern, but we have a few places where this is clearly visible.

In the screenshot, you can find the additional leading space that is introduced with line-height 1.2. This makes the cell content appear slightly aligned to the bottom instead of center.

The clear visual difference appears when we enter and exit edit mode, causing a slight jerk in the text, as shown in the below video:

Screen.Recording.2022-01-29.at.2.08.28.AM.mov

The other place where it appears is the header images (which are temporary at this point).

I've added a commit 40e9b2a which fixes these immediately visible places. We could improve upon the css during our styling update tasks. Please take a look at the commit and feel free to merge.

[seancolsen]

@pavish I think the slight jerk in text is better than table names appearing to display underscore characters as space characters. So I'm going to merge this.

If we're going to get particular about spacing, I think we ought to ship a web font to the browser. I can certainly see the appeal of using system fonts. But I'd lean towards being explicit about the font -- for the same reason we want to be explicit about the line height -- because we want predictable spacing across different platforms.