-
Notifications
You must be signed in to change notification settings - Fork 9
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
Refactor: agency logo width and height #1632
Conversation
Ah, thanks for catching that. It seems like we're basically relying on the widths being equal and using the largest logo's width to center them. So should we set SacRT back to the MST width? And when we add SBMTD's logo, which is the widest, we'll have to set MST and SacRT's logo widths to that one? Or do you think we should do something in CSS to make them centered? |
@angela-tran I think actually, Design should be giving us custom logo files for both Desktop and Mobile, with extra padding if it's a logo like SacRT's - which is narrower than MST/SBMTD. Like this Figma export has 9px on the left and right: I think if Design always customized logos for Dev, and resized/centered/cropped/whatever-is-necessary to get all agency logos to fit 1 dimension - then we won't need this code at all. It would be Design's responsibility to come up with the size of the logo and how the logo is centered within a specified rectangular space that never changes for Desktop and Mobile. In other words, Devs should never go to an agency's site or an email or Google Drive to get an agency's logo file and upload that for use on the app -- this should always go through Design and Design needs to be resizing, cropping and centering the logo for use on the agency selector page for Desktop and Mobile. |
@machikoyasuda That makes sense to me and aligns I think with what @indexing and @srhhnry have been talking about with figuring out max-dimensions/character lengths for the Agency Card component.
I agree that this would be a good process to follow going forward. I don't think we (product, design, and dev) ever discussed that this is the expectation. Do we block this until product/design gets us logos they've edited? |
Update: Machiko let me know that Andy has already edited the logos in Figma and that they can be exported from there for use in this PR and #1631.
Looks like those screenshots were from a Figma board called "Copy Sync Demo" so I don't think these values are the ones that design intends for us to go with. Seems like the ones Sarah posted in #1632 (comment) are the ones currently. |
Awaiting completion of #1444 |
Hey! So per issue #1444 we've worked out a single dimension for mobile and desktop logos. The summary of that issue thread is that agency logos can be different shapes. Also, to make the Figma component work in a standardized way I have to manipulate the shape a bit (long story, but "hugging" components to make them look good breaks their fixed width/height--this might be a me problem). Anyway, you can find the dimensions on the components page under Icons + Logos or as seen here: Let me know if that works for you all, or if we need to adjust the sizing/dimension/approach |
@srhhnry We're gonna need 2 logo files for each agency, with the agency's logo files cropped to those now standard dimensions. The desktop logos in the styleguide aren't exportable in the rectangular size. There's no layer with the file in it in that dimension. The mobile logos are off by 1px If you could upload those files to the GitHub ticket or correct it on Figma, we just need the files in the correct dimensions one way or another. |
@srhhnry While there are a likely a variety of approaches to get agency assets for dev that export at a consistent dimension, I found this structure works.
Since the logo and the region are fixed, there is no need for auto layout. (Auto layout might, however, be useful at the card level when nesting the logo region component in the agency selector card component even though its dimensions remain fixed.) |
Per @indexing's suggestion on approach I added the mobile files to Figma as frames that conform to the standard sizing. So both. the desktop sizing Andy did as well as the mobile sizing is now complete. You can find them on the Components page and they look like this: |
@srhhnry Yay thank you. Can you do SBMTD logo too? We will need that for this upcoming PR. @angela-tran is getting ready to add SBMTD to the app. |
I actually don't have the official logo from SBMTD (and am not sure where we are in the intake process with them) @o-ram do you know if we've gone through that questionnaire with SBMTD and if so, did they send over their official logo? Otherwise we're just pulling the image from their website, which might be fine but we'd want them to weigh in on it at least. Edit: Nevermind @o-ram! |
Updated! |
add new logo files from design that have correct dimensions
81f65b4
to
29d1fc9
Compare
Part of #1588
This PR updates the agency selector to use the new width and height that product and design came up with so that all logos fit within them. This PR also updates the MST and SacRT logos to use the files that design created to have correct padding.
(This PR previously refactored the modal to use separate templates for each agency, which prompted a lot of the discussion below.)