Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding new Bynder logo #1308

Merged
merged 10 commits into from
Oct 3, 2023
Merged

Adding new Bynder logo #1308

merged 10 commits into from
Oct 3, 2023

Conversation

webknit
Copy link
Contributor

@webknit webknit commented Oct 2, 2023

  • Adding new SVG
  • Restricting max width specifically for bynder logo
  • Applying border radius

@AmeeMorris
Copy link
Contributor

Whats your plan to featureflag this? might be worth having both in GUI then we can pass in a prop (controlled by a flag) from webapp that will switch between the two

AmeeMorris
AmeeMorris reviewed Oct 2, 2023
View reviewed changes
lib/Logo/index.tsx Outdated Show resolved Hide resolved
@webknit
Copy link
Contributor Author

webknit commented Oct 2, 2023 •
edited
Loading

Whats your plan to featureflag this? might be worth having both in GUI then we can pass in a prop (controlled by a flag) from webapp that will switch between the two

Good point @AmeeMorris. I never considered 🙀. I have added old component <LogoGC/> and will conditionally swap based on flag. Then when remove flags/conditionals and simply delete this old component from gather-ui. My logic sound ok?

@webknit webknit requested a review from AmeeMorris October 2, 2023 14:19
AmeeMorris
AmeeMorris reviewed Oct 2, 2023
View reviewed changes
lib/LogoGC/index.tsx
<img src={props.url} alt={props.alt} className="logo__image" />
) : (
<span className="logo__image">
<LogoPath />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This points to the same svg as the new bynder one, we need the old svg in place :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added old SVG back and also added a new story so it's clear what's what.

@webknit webknit requested a review from AmeeMorris October 2, 2023 15:06
@webknit webknit merged commit fdee243 into main Oct 3, 2023
2 checks passed
@webknit webknit deleted the feature/new-bynder-logo branch October 3, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timbryandev timbryandev timbryandev approved these changes

@lewishankinson lewishankinson Awaiting requested review from lewishankinson

@AmeeMorris AmeeMorris Awaiting requested review from AmeeMorris

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@webknit @AmeeMorris @timbryandev