Skip to content

Use pre-processed images everywhere #151

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.

Sign up for GitHub

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

Merged
pftg merged 3 commits into from
Sep 27, 2024
Merged

Use pre-processed images everywhere #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f123238
move images in css to assets and use dynamic css with fingerprinted i…
AucT Sep 26, 2024
a9707b8
Merge branch 'master' into remove-not-needed-static
pftg Sep 27, 2024
4a838ca
test: updates screenshots
pftg Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified tests/fixtures/screenshots/blog/index.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/fixtures/screenshots/homepage/_clients.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/fixtures/screenshots/mobile/blog/post.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/fixtures/screenshots/mobile/nav/hamburger_menu.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion tests/mobile_site_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_top_bar_hamburger_menu

open_mobile_menu

assert_matches_screenshot "nav/hamburger_menu", wait: 1, stability_time_limit: 0.025
assert_matches_screenshot "nav/hamburger_menu", wait: 2, stability_time_limit: 0.025
end

def test_top_bar_hamburger_menu_services
Expand Down
4 changes: 2 additions & 2 deletions tests/site_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_homepage_sections
sections.each do |section_id|
scroll_to find("##{section_id}")
sleep 1.5
assert_matches_screenshot "homepage/_#{section_id}", wait: nil, stability_time_limit: nil
assert_matches_screenshot "homepage/_#{section_id}", wait: 3, stability_time_limit: 0.25
end
end

Expand Down Expand Up @@ -157,7 +157,7 @@ def test_free_consultation
def test_not_found
visit "/404.html"

assert_matches_screenshot "404", wait: 5, stability_time_limit: 3
assert_matches_screenshot "404", wait: 5, stability_time_limit: 0.25
end

private
Expand Down
25 changes: 25 additions & 0 deletions themes/beaver/assets/css/586-dynamic.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.pp-gf-content .pp-gf-inner .gform_confirmation_message i::before {
content: "";
width: 70px;
height: 70px;
display: block;
background-image: url({{ (resources.Get "img/icons/theme/confirmation-icon.svg" | fingerprint).RelPermalink }});
background-repeat: no-repeat;
background-size: contain;
}

.jt-reviews-box .pp-reviews-wrapper .pp-swiper-button:hover span::before {
background-image: url({{ (resources.Get "img/icons/theme/hover-arrow-icon-hover.svg" | fingerprint).RelPermalink }});
}

.cs-testimonial .cs-testimonial-wrapper::before {
content: "";
display: block;
width: 60px;
height: 60px;
background-image: url({{ (resources.Get "img/testimonial-quote.png" | fingerprint).RelPermalink }});
background-size: contain;
position: absolute;
top: 0;
left: 0;
}
Comment on lines +15 to +25

@coderabbitai coderabbitai Bot Sep 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

The suggested changes to use SVG and improve responsiveness have not been fully implemented.

  • testimonial-quote.svg does not exist in the assets/img/ directory.
  • themes/beaver/assets/css/586-dynamic.css still references testimonial-quote.png.
🔗 Analysis chain

Consider using SVG and improving responsiveness for the testimonial quote icon.

While the use of dynamic URL with fingerprinting is good, there are a few areas for improvement:

  1. The quote icon is a PNG file. Consider using an SVG for better scalability.
  2. The fixed dimensions might not be responsive on all screen sizes.

Here's a suggested improvement:

 .cs-testimonial .cs-testimonial-wrapper::before {
     content: "";
     display: block;
-    width: 60px;
-    height: 60px;
-    background-image: url({{ (resources.Get "img/testimonial-quote.png" | fingerprint).RelPermalink }});
+    width: 10%; /* Use percentage for responsiveness */
+    padding-bottom: 10%; /* Maintain aspect ratio */
+    background-image: url({{ (resources.Get "img/testimonial-quote.svg" | fingerprint).RelPermalink }}); /* Use SVG instead of PNG */
     background-size: contain;
     position: absolute;
     top: 0;
     left: 0;
 }

This change aligns with the PR objective of using pre-processed images (SVG) and improves the responsiveness of the design.

To ensure we're not missing any PNG files that should be converted to SVG, let's run the following check:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find PNG files in the assets directory that might be candidates for SVG conversion

echo "PNG files in the assets directory:"
fd -e png . assets

Length of output: 1283

Script:

#!/bin/bash
# Check if testimonial-quote.svg exists in assets/img
echo "Checking for testimonial-quote.svg in assets/img:"
fd -e svg 'testimonial-quote.svg' assets/img

# Check if themes/beaver/assets/css/586-dynamic.css references testimonial-quote.png
echo "Checking for references to testimonial-quote.png in themes/beaver/assets/css/586-dynamic.css:"
grep 'testimonial-quote\.png' themes/beaver/assets/css/586-dynamic.css

Length of output: 528

🧰 Tools
🪛 Biome

[error] 20-20: Unexpected value or character.

Expected one of:

(parse)

[error] 20-20: Unexpected value or character.

Expected one of:

(parse)

[error] 20-20: Unexpected value or character.

Expected one of:

(parse)

[error] 20-20: Unexpected value or character.

Expected one of:

(parse)

[error] 20-20: Expected a qualified rule, or an at rule but instead found '});'.

Expected a qualified rule, or an at rule here.

(parse)

[error] 21-21: expected , but instead found ;

Remove ;

(parse)

[error] 22-22: expected , but instead found ;

Remove ;

(parse)

[error] 23-23: Unexpected value or character.

Expected one of:

(parse)

23 changes: 0 additions & 23 deletions themes/beaver/assets/css/586.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,6 @@ body a, body .fl-button, body .pp-button, body button, body input[type="submit"]
margin-bottom: 20px;
}

.pp-gf-content .pp-gf-inner .gform_confirmation_message i::before {
content: "";
width: 70px;
height: 70px;
display: block;
background-image: url("/wp-content/uploads/2022/10/confirmation-icon.svg");
background-repeat: no-repeat;
background-size: contain;
}

.gform_wrapper .gform_heading .gform_required_legend {
display: none !important;
Expand Down Expand Up @@ -542,9 +533,6 @@ footer .footer-menu.careers-text .fl-rich-text p:last-child a strong {
transform: rotate(-180deg);
}

.jt-reviews-box .pp-reviews-wrapper .pp-swiper-button:hover span::before {
background-image: url("/wp-content/uploads/2022/09/hover-arrow-icon-hover.svg");
}

.jt-contact-list ul.pp-list-items li:last-child {
padding-bottom: 0 !important;
Expand Down Expand Up @@ -970,17 +958,6 @@ footer .footer-menu.careers-text .fl-rich-text p:last-child a strong {
}
}

.cs-testimonial .cs-testimonial-wrapper::before {
content: "";
display: block;
width: 60px;
height: 60px;
background: url("/wp-content/uploads/2022/10/testimonial-quote.png") no-repeat;
background-size: contain;
position: absolute;
top: 0;
left: 0;
}

@media (max-width: 860px) {
.cs-testimonial .cs-testimonial-wrapper::before {
Expand Down
Loading