-
Notifications
You must be signed in to change notification settings - Fork 27
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
Windows support for Malicious website protection #1423
Conversation
✅ Deploy Preview for content-scope-scripts ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
[Beta] Generated file diffTime updated: Sun, 16 Feb 2025 19:03:37 GMT Integration
File has changed Apple
File has changed New Files
❌ File only exists in new changeset |
1d86929
to
2ec637b
Compare
2ec637b
to
52d7477
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few UI issues in a comment - namely the radius of the corners, and the padding for the "Advanced Info" not looking right
8fd1c2d
to
a934d68
Compare
Hey, @RendijsSmukulis
![]() ![]() I’ve recently rebased to main which may have fixed the preview and could explain what you saw earlier. Let me know if you can still reproduce it. |
a934d68
to
4bfc525
Compare
49eff55
to
cd85af2
Compare
(as discussed in MM) the "Advanced margin issue" was caused by "Animate controls and elements inside windows" setting disabled on my machine, which was picked up by The fixed version works great on my machine. |
special-pages/pages/special-error/app/components/AdvancedInfo.module.css
Show resolved
Hide resolved
8bda22f
to
9033548
Compare
9033548
to
bd506e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question on the amount of screenshot - since we're not verifying in CI yet, is it worth cutting them down to 1 per layout?
for example, phishing-warning-advanced-reduced-motion-macos-darwin.png and
ssl-expired-cert-reduced-motion-macos-darwin.png - the only difference is text I think? if so, it might be better suited to test those variants with faster more reliable things, like DOM/aria tests.
That could cut down on the amount of screenshots for now until we formalize how we want to capture this stuff going forward. I'm slightly worried that if we do this level of screenshot testing for all features it's going to get into the thousands very quickly.
I think using screenshots to verify a particular layout is great - but please have a look if any of them in this PR can be de-duplicated?
special-pages/pages/special-error/integration-tests/special-error-screenshots.spec.js
Show resolved
Hide resolved
@shakyShane I’ve cut down the screenshots to the essentials. Thanks for the tip. |
Asana Task/Github Issue: https://app.asana.com/0/72649045549333/1208745889841029/f
Description
Previews:
Figma
Testing Steps
Checklist
Please tick all that apply: