feat(ONYX-1974): add new layout for signup modal #16455
Conversation
#5483 Bundle Size — 9.51MiB (+0.02%).4bb95a5(current) vs cf3fff0 main#5480(baseline) Warning Bundle contains 34 duplicate packages – View duplicate packages Bundle metrics
|
| Current #5483 |
Baseline #5480 |
|
|---|---|---|
 Initial JS |
3.7MiB(+0.06%) |
3.69MiB |
 Initial CSS |
0B |
0B |
 Cache Invalidation |
37.53% |
37.45% |
| Chunks |
102 |
102 |
| Assets |
105 |
105 |
| Modules |
5939(+0.03%) |
5937 |
| Duplicate Modules |
590 |
590 |
| Duplicate Code |
4.46% |
4.46% |
| Packages |
297 |
297 |
| Duplicate Packages |
33 |
33 |
Bundle size by type 1 change 1 regression
| Current #5483 |
Baseline #5480 |
|
|---|---|---|
| JS |
9.37MiB (+0.02%) |
9.37MiB |
| Other |
144.27KiB |
144.27KiB |
Bundle analysis report Branch araujobarret/feat/signup-modal-i... Project dashboard
Generated by RelativeCI Documentation Report issue
| width: MODAL_WIDTH, | ||
| height: 2030, | ||
| src: "https://files.artsy.net/images/signup03.webp", | ||
| }, |
There was a problem hiding this comment.
These images are extremely low quality and have border artifacts. Also you don't need to convert to webp for their source.
There was a problem hiding this comment.
I got new ones from Design, will update in the PR
araujobarret
force-pushed
the
araujobarret/feat/signup-modal-image
branch
from
6a790ad to
540ce70
Compare
| const IMAGE = { | ||
| width: MODAL_WIDTH, | ||
| height: 2030, | ||
| src: "https://d/images/2x_Evergreen-Artist-Page-Sign-Up-Modal.jpg", |
There was a problem hiding this comment.
question: is there a reason why this image is not .webp?
There was a problem hiding this comment.
The proxy handles the webp conversion (which is dependent on if the browser supports it: content negotiation). These should be high quality jpgs or pngs (if they are graphics).
There was a problem hiding this comment.
Pushed changes with a higher res(3x) handed off from Design, and in the PNG format.
| } | ||
| ` | ||
|
|
||
| const COLUMN_WIDTH = MODAL_WIDTH / 2 - 20 |
There was a problem hiding this comment.
question: can we use something like space(2) instead of 20 here?
There was a problem hiding this comment.
You can if you pull it out of the theme. It's safe to access spacing values directly since those don't change between themes.
There was a problem hiding this comment.
I've used it from there and had to parse the 20px string to int, doesn't look so nice though, but it's safer if we change our spacings at some point in time
araujobarret
force-pushed
the
araujobarret/feat/signup-modal-image
branch
from
08f17f5 to
a0b106f
Compare
araujobarret
force-pushed
the
araujobarret/feat/signup-modal-image
branch
from
a0b106f to
4bb95a5
Compare
|
Can I proceed with this PR, or are there any further requests? The only thing I need to check is Integrity, gonna run it against my branch to make sure I'm not breaking it |
| { | ||
| width: MODAL_WIDTH, | ||
| height: IMAGE_HEIGHT, | ||
| src: "https://files.artsy.net/images/signup-01.png", |
There was a problem hiding this comment.
This image is still very low quality and has a visible border
| { | ||
| width: MODAL_WIDTH, | ||
| height: IMAGE_HEIGHT, | ||
| src: "https://files.artsy.net/images/signup-02.png", |
The type of this PR is: Feat
This PR solves ONYX-1974
Description
Implements the left column in the signup modal, displaying 3 images in a carousel.
Screen.Recording.2025-12-05.at.17.44.19.mov