Add authority logos at bottom of results #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Description This isn't 100% exactly as designed. The design calls for a white background that extends horizontally beyond the borders of the incentive grid. That doesn't work with how this embed element is implemented: it just fills 100% of the width of its container, so nothing can extend beyond the grid boundaries. Not sure if what I've done is an acceptable end state, or if we need to build in a way to limit the width of the incentive grid without limiting the width of the logos background. ## Test Plan See screenshot. Try with different utility selections to make sure their logos come up. Check narrow and medium layouts to check for appropriate layout.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
RandomEtc
reviewed
Sep 26, 2023
| export const authorityLogosStyles = css` | ||
| .authority-logos { | ||
| width: 100%; | ||
| max-width: 1280px; |
There was a problem hiding this comment.
We should probably have a variable for this and ensure we're doing maximum widths consistently on all the elements?
RandomEtc
approved these changes
Sep 26, 2023
There was a problem hiding this comment.
LGTM. Passing remark on CSS can be rolled forward into discussion about Tailwind or whatever. I made a ticket for that here: https://app.asana.com/0/1205057814651000/1205587645865467/f
|
Design wise, let's flag this with Spenser. I think it's a good callout that this might not work great in an embed context. Perhaps we should use a card container like the form? |
|
Yeah, I'm OK with deferring a solution to this, at least until Spenser is back. I think it doesn't look egregious as-is. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This isn't 100% exactly as designed. The design calls for a white
background that extends horizontally beyond the borders of the
incentive grid. That doesn't work with how this embed element is
implemented: it just fills 100% of the width of its container, so
nothing can extend beyond the grid boundaries. Not sure if what I've
done is an acceptable end state, or if we need to build in a way to
limit the width of the incentive grid without limiting the width of
the logos background.
Test Plan
See screenshot. Try with different utility selections to make sure
their logos come up. Check narrow and medium layouts to check for
appropriate layout.