department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
283 stars 204 forks source link

[Spike] Determine best approach for ensuring content font size #92090

Closed JunTaoLuo closed 1 week ago

JunTaoLuo commented 2 months ago

Background/Request

As part of various work in VAOS, we found many instances where the content's font size in incorrect. All default text should be 16.96px but we often display content at the 16px font size, see discussion here. The technical reason is that only some semantic tags are styled with the correct font size by default and all others (e.g. span) do not have the correct font size applied.

Goal

The ultimate goal is to ensure all VAOS content has the correct font size applied. However, given the large amount of text and elements this affects, we need to determine the best way to approach this. This ticket tracks the exploration of which approach to take in the end. Here are a few ideas to get started:

  1. Come up with a list of HTML elements that are currently missing the correct font size and ask Platform to include them in the semantic tags list.
  2. Modify the VAOS top level css file to apply the correct font size
  3. Individually wrap text with a semantic tag (e.g. <p>) to ensure the correct styling is applied
  4. Explore how other teams have dealt with this issue and see if those approaches works for us

Requirements to Consider

Given the large amount of affected content in VAOS and how this is not an issue specific to our app, consider how this impacts other teams/apps and how future content updates/additions should be handled.

Tasks

Time Box

_ hours

Definition of Done


How to configure this issue

Bren22va commented 1 month ago

Hey team! Please add your planning poker estimate with Zenhub @cferris32 @jenniemc @JunTaoLuo @ryanshaw @simiadebowale @vbahinwillit

jenniemc commented 1 week ago

The body font size is 16px and its assignment is coming from va.scss. The most expedient fix is to create our own body tag in vaos.scss. Since we are inheriting the body tag of 16px, we can assign our body tag to use the relative size of 1.06rem. This will fix the body tag and all non semantic tags such as div and span to have default font size 16.96px with the least amount of disruption.

body {
  font-size: 1.06rem; // same as 16.96px
}

After adding the body tag to src/applications/vaos/sass/vaos.scss there are still a few minor updates to clean up the display.

Remove all occurrence of “vads-u-font-size—sm” (15px)

The majority of “vads-u-font-size—sm” are coming from the UIShcema. Removing the class name will default the font to 16.96px because they are wrapped within the form/label tag.

The exception to the rule to keep “vads-u-font-size—sm” are the two files: (1) src/applications/vaos/new-appointment/components/FormLayout.jsx
(2) src/applications/vaos/covid-19-vaccine/components/FormLayout.jsx The FormLayout has the text "NEW APPOINTMENT/REQUEST COMMUNITY CARE "in all uppercase and is intended to have a smaller font size.

Change from “vads-u-font-size—h5” (15px) to “vads-u-font-size—h4” (17.008px) And also may want to add “vads-u-margin-bottom—0p5” to give more bottom space below the heading

Review the use of “vads-u-font-size--base” (16px) Since we want the normalization size of 16.96px. This includes removing var(--base-font-size) from src/applications/vaos/new-appointment/sass/styles.scss

.vaos-appts__block-label {
  margin: 0;
  margin-bottom: 0.25rem; // still computes to 4px
  font-size: var(--base-font-size);  // can delete
  font-family: var(--font-source-sans);  // can delete
  line-height: 1.5em;
}

Review the use of “vads-u-font-family--sans” Since we are inheriting the font family already

~~Optional May want to wrap the <address> semantic tag in sections that displays address and phone information~~

jenniemc commented 1 week ago

Per conversation with @outerpress, please exclude the Optional suggestion.

outerpress commented 1 week ago

@ldelacosta Jennie ran me through this and we decided not to pursue the optional suggestion, but everything else is good to go

ldelacosta commented 1 week ago

Thanks @outerpress . Closing out the ticket.

jenniemc commented 1 week ago

@ldelacosta, One ticket should take care of all the font updates.