Frontend: App Branding! #1592
Merged
Frontend: App Branding! #1592
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
Shrinks99
added
enhancement
emma-sg
reviewed
Mar 13, 2024
SuaYoo
force-pushed
the
frontend-browsertrix-branding
branch
from
bd7b057 to
9df5a50
Compare
SuaYoo
approved these changes
Mar 19, 2024
There was a problem hiding this comment.
Nice!! I fixed a few merge conflicts, looks good to go.
Out of scope, potential follow-up: since "Browsertrix" is now bold and darker, it does take a lot of prominence in the app, when maybe the org name should be more prominent? We don't currently show the org name in the app bar if a user only belongs to one org, maybe we could display it as [Browsertrix logo mark] | [Org name].
SuaYoo
added a commit
that referenced
this pull request
Mar 19, 2024
Addresses failing test in #1592 by fixing asset imports in unit tests. Unit tests now import an empty string for all assets--note: if we want to test actual asset content, will need to update this config.
- Fixes a few instances where blue is hardcoded and shouldn't be. - Fixes a few instances where _primary_ is hardcoded and shouldn't be!
This is a destructive action and should be red!
SuaYoo
force-pushed
the
frontend-browsertrix-branding
branch
from
0f99cd4 to
834a0e9
Compare
At some point the name got mixed up, this fixes it 😅
Spacing update, suggestion applied from Emma
Apparently this is fine and you can do it as long as the image has an alt tag (which this does!)
|
It looks like the logomark in the image isn't vertically centered. There's more space beneath than above. |
ikreymer
approved these changes
Apr 18, 2024
### Changes App bar and org top-level navigation is full-width. Also, adds background color to app bar to differentiate it from the org nav.
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.
Closes #424
Changes
primaryvsbluefor elements where it shouldn't be the samedangerbutton variantScreenshots!