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

UI: Increase base font size #23994

Merged
merged 16 commits into from
Nov 13, 2023
Merged

Conversation

hellobontempo
Copy link
Contributor

@hellobontempo hellobontempo commented Nov 3, 2023

This PR increases the on-prem UI base font size from 14px to 16px.

To accomplish this, sizing variables were first audited and updated to consistently use pixels for padding and margin values. In addition, this PR addresses some minor cleanup intended to make Helios practices easier to iteratively address in the future.

  1. rename spacing variables with numerical values, ex: $spacing-xxs (which is 4 pixels) is now $spacing-4
  2. fixes toggle component, which was unaligned with container

toggle before + after
Screenshot 2023-11-03 at 11 29 18 AM

  1. replace all $size- variables for margin or padding with equivalent pixel value. To preserve original spacing, pixel values were calculated using the 14px as the base rem size.

Comments in screenshot are the base-14 pixel values of each size variable:
Screenshot 2023-11-03 at 12 08 15 PM

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Nov 3, 2023
@@ -46,12 +46,12 @@
id="fileUploadToggle"
@type="checkbox"
name="fileUploadToggle"
class="switch is-rounded is-success is-small"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted switch css to consistently use toggle

before + after

Screenshot 2023-11-03 at 1 23 33 PM

Copy link

github-actions bot commented Nov 3, 2023

Build Results:
All builds succeeded! ✅

@@ -5,7 +5,7 @@

.env-banner {
align-self: center;
border-radius: $size-1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted this variable because it was only used in one place

@@ -11,7 +11,7 @@
{{#each (filter-by "category" category this.mountTypes) as |type|}}
<SelectableCard
id={{type.type}}
class="has-top-padding-m has-text-centered small-card"
class="has-top-padding-l has-text-centered small-card"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to increase when font sizes increased
Screenshot 2023-11-03 at 9 28 50 AM

@@ -32,8 +32,6 @@
<div>
<Toggle
@name={{concat @attr.name "-validation-toggle"}}
@status="success"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there were no alternatives to the component, so <Toggle> now renders these classes by default

@@ -9,8 +9,6 @@
<ToolbarFilters>
<Toggle
@name="json"
@status="success"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-11-03 at 2 43 55 PM

@@ -80,7 +80,7 @@ object {

html {
background-color: $white;
font-size: 14px;
font-size: $base-font-size; // 16px
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this where the font size increase happened

Copy link
Contributor Author

@hellobontempo hellobontempo Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some changes here because the toggle before was just slightly off

before (left) + after (right)

Screenshot 2023-11-03 at 11 29 18 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-11-03 at 12 48 29 PM

@kiannaquach
Copy link
Contributor

I noticed that the placeholder text gets cut off for Entities filter dropdown:
Screenshot 2023-11-03 at 2 58 10 PM

The SearchSelect search icon is misaligned with the text now that the size increased:
Screenshot 2023-11-03 at 3 53 08 PM

With the larger font sizes I think SearchSelect and buttons get larger on Overview components (I noticed this in pki overview):
Screenshot 2023-11-03 at 3 55 14 PM

@@ -6,7 +6,7 @@
{{#unless @uploadOnly}}
<div class="level is-mobile">
<div class="level-left">
<label for="input-{{this.elementId}}" class="is-label" data-test-text-file-label>
<label for="input-{{this.elementId}}" class="has-text-weight-semibold" data-test-text-file-label>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-11-03 at 4 03 43 PM

@@ -40,13 +38,6 @@ export default class ToggleComponent extends Component {
get safeId() {
return `toggle-${this.name.replace(/\W/g, '')}`;
}
get inputClasses() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only ever saw small and success passed in, so removed this getter

@hellobontempo
Copy link
Contributor Author

hellobontempo commented Nov 6, 2023

@kiannaquach - good catch with the entities input! I'll take a look at the placeholder text.

For the overview cards - the larger button is not because of the font size change but because search select has a bottom margin and the button stretches to fill the space. This will be fixed when the buttons are replaced - I've mentioned it in that ticket to address the buttons stretching to fill

The SearchSelect search icon is misaligned with the text now that the size increased: Screenshot 2023-11-03 at 3 53 08 PM

With the larger font sizes I think SearchSelect and buttons get larger on Overview components (I noticed this in pki overview): Screenshot 2023-11-03 at 3 55 14 PM

@hellobontempo hellobontempo marked this pull request as ready for review November 6, 2023 17:18
Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I did a lot of clicking around and the only thing I found that happens on this branch and not main is in the toolbar filters on clients counts. If you add a namespace it adds a Y scrollbar. It's a non-blocking find.
image

@hellobontempo
Copy link
Contributor Author

Thank you @Monkeychip - updated toolbar by increasing the height for toolbar-scroller
Screenshot 2023-11-13 at 1 03 05 PM

@hellobontempo hellobontempo added this to the 1.16.0-rc1 milestone Nov 13, 2023
@hellobontempo hellobontempo enabled auto-merge (squash) November 13, 2023 21:17
@hellobontempo hellobontempo merged commit 7f03393 into main Nov 13, 2023
69 checks passed
@hellobontempo hellobontempo deleted the ui/VAULT-20096/increase-base-font-size-2 branch November 13, 2023 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants