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

Prune away unnecessary sass #719

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Prune away unnecessary sass #719

wants to merge 2 commits into from

Conversation

jcoyne
Copy link
Contributor

@jcoyne jcoyne commented Jun 5, 2024

No description provided.

@jcoyne jcoyne force-pushed the prune-sass branch 7 times, most recently from 184d650 to 50019c9 Compare June 5, 2024 18:47
@taylor-steve
Copy link
Contributor

Locally it looks like we've lost the navbar logo:

Screenshot 2024-06-05 at 12 39 25 PM

@jcoyne jcoyne force-pushed the prune-sass branch 5 times, most recently from ebee4c7 to ffd2f37 Compare June 5, 2024 21:42
@taylor-steve
Copy link
Contributor

The only other thing I see is it seems we've lost the black text here on the active facet header:

Screenshot 2024-06-05 at 3 46 01 PM

https://sul-arclight-demo.stanford.edu/collections

@jcoyne jcoyne force-pushed the prune-sass branch 13 times, most recently from 5f33409 to 17ce109 Compare June 10, 2024 14:55
Copy link
Contributor

@corylown corylown left a comment

Choose a reason for hiding this comment

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

Many of the issues below seem specific for Firefox 115.11.0esr (which is what gets installed by Stanford's Self Service app). I will review again with Firefox 126.0.1

General

Hamburger menu

icon should be rgb(255, 255, 255)

main:
hamburger-main

prune-sass
hamburger-prune-sass

Header links

link color should be rgb(255, 255, 255)
active link color should be rgb(255, 255, 255)
hover state should be underlined
should be separated by a dot

main:
header-links-main

prune-sass:
header-links-prune-sass

Masthead h1

--al-masthead-title-size should be 36px at larger screen widths not 40px

main:
masthead-h1-main

prune-sass:
masthead-h1-prune-sass

--al-masthead-title-size should be 24px at smaller screen widths not 32px

main:
masthead-h1-smaller-screen-size-main

prune-sass:
masthead-h1-smaller-screen-size-prune-sass

hover state should be underlined

main:
masthead-h1-hover-state-main

prune-sass:
masthead-h1-hover-state-prune-sass

Landing Page

Search button background

should be rgb(255, 255, 255)

main:
landing-page-search-button-main

prune-sass:
landing-page-search-button-prune-sass

Grey masthead alignment

Should align with yellow and green backgrounds above

main:
masthead-padding-main

prune-sass:
masthead-padding-prune-sass

Cards

Card buttons should both be the same width and span almost the entire width of the containing card-body div

main:
request-find-main

prune-sass:
request-find-prune-sass

Card images should sit above the card div
Card h3 font weight should be 700

main:
grid-image-layout-main

prune-sass:
grid-image-layout-prune-sass

h1 & h2 text (Find Archival Materials, About this site)

Should be font-family "Source Serif 4", serif
font weight should be 700
font size should be 36px

main:
about-site-main

prune-sass:
about-site-prune-sass

Masthead image caption links

Caption links should be white
Caption links hover state should be underlined
Caption link should be font-weight 600

main:
featured-main

prune-sass:
featured-prune-sass

Browse links

Should be horizontal and separated by a dot the same color as the link text

main:
browse-links-alignment-main

prune-sass:
browse-links-alignment-prune-sass

Repositories Page

Cards

Card buttons should both be the same width and span almost the entire width of the containing card-body div

main:
browse-button-main

prune-sass:
browse-button-prune-sass

Card images should sit above the card div

main:
browse-repos-font-and-image-align-main

prune-sass:
browse-repos-font-and-image-align-prune-sass

Bottom of card image and card h2 should align with the adjacent card
Card h3 font weight should be 700

main:
image-text-alignment-01-main
image-text-alignment-02-main

prune-sass:
image-text-alignment-01-prune-sass
image-text-alignment-02-prune-sass

Cards and footer should not overlap

main:
bottom-row-footer-main

prune-sass:
bottom-row-footer-prune-sass

Results Page

Icons

Bookmark icon color should be rgb(1, 66, 64)
Result type icon color should be rgb(1, 66, 64)

main:
bookmark-icon-main
result-type-icon-main

prune-sass:
bookmark-icon-prune-sass
result-type-icon-prune-sass

Digital content

Design reference: #602

font weight should be 700
should have solid left border, color rgb(1, 66, 64), 8px wide

main:
digital-content-main

prune-sass:
digital-content-prune-sass

Pagination

Previous and Next buttons should have background color rgb(1, 66, 64)

main:
pagination-main

prune-sass:
pagination-prune-sass

Applied facets

applied-filter Remove facet button hover state background should be rgb(1, 66, 64)

main:
remove-facet-main

prune-sass:
remove-facet-prune-sass

selected facet-level-header background should be rgb(212, 209, 209)

main:
selected-facet-main

prune-sass:
selected-facet-prune-sass

Search widget

selection color in dropdown should be rgb(1, 66, 64)

main:
sort-dropdown-main

prune-sass:
sort-dropdown-prune-sass

hover state should be underlined

main:
sort-dropdown-hover-state-main

prune-sass:
sort-dropdown-hover-stage-prune-sass

Result links

hover state should be underlined (this looks like a general problem)

main:
link-hover-state-main

prune-sass:
link-hover-state-prune-sass

Collection/Component Page

Collection panel

Padding needs attention
Collection overview link list should have disc list style in the same color as the links
More info button should not have an icon and the background should be white
background color should be rgb(235, 245, 241)

main:
collection-info-left-padding-main
collection-info-padding-main

prune-sass:
collection-info-left-padding-prune-sass

Browse hierarchy panel

Expand button should have background color rgb(255, 255, 255)
Expand button link hover state should be underlined and background rgb(1, 66, 64)

main:
expand-button-main

prune-sass:
expand-button-prune-sass

Digital content

Design reference: #602

font weight should be 700
should have solid left border, color rgb(1, 66, 64), 8px wide
height should be 32px

main:
digital-content-main

prune-sass:
digital-content-prune-sass

Search controls

Search label should not look like a button. The background should match navbar-search rgb(46, 45, 41)
Search label font weight should be 700
There should be even spacing between each segment of the search selects/inputs

main:
record-view-search-controls-main

prune-sass:
record-view-search-controls-prune-sass

@jcoyne
Copy link
Contributor Author

jcoyne commented Jun 10, 2024

@corylown aside from one or two of those, I don't see anything like that. Maybe do bin/rails assets:clobber first.

@corylown
Copy link
Contributor

The following are still issues on Firefox 126.0.1:

  • hamburger menu icon color
  • document type icon color
  • bookmark icon color on index page (it's fine on the show page)
  • remove facet hover state background color

@jcoyne jcoyne force-pushed the prune-sass branch 4 times, most recently from e2eedfa to 06aec64 Compare June 12, 2024 12:13
@jcoyne
Copy link
Contributor Author

jcoyne commented Jun 12, 2024

@corylown I believe those issues are resolved.

@jcoyne jcoyne force-pushed the prune-sass branch 4 times, most recently from 0783f94 to 473525d Compare June 14, 2024 15:21
@corylown
Copy link
Contributor

corylown commented Nov 4, 2024

@jcoyne now that Stanford is on Firefox 128.4.0esr I think this could be rebased and I'd review again.

Copy link
Contributor

@corylown corylown left a comment

Choose a reason for hiding this comment

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

Looks great. See inline comment about the palette import.

@@ -1,28 +1,112 @@
// Entry point for your Sass build

@import "palette";
@import "bootstrapVariables";
@import url("palette");
Copy link
Contributor

Choose a reason for hiding this comment

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

I get a 404 for this import when it's wrapped in the url() helper.

Suggested change
@import url("palette");
@import "palette";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants