-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
compact mapbox wordmark on narrow maps #6472
Conversation
cc6f7b1
to
1e16e24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this @andrewharvey!
2 questions –
where is the SVG asset you've inlined from? Just want to make sure we're using the most up to date version and check with our design team.
is the 250 cutoff from API GL only? We're using 640px as the cutoff between compact and normal attribution, and I wonder if it makes sense to stay consistent there.
I took the current inlined svg for the normal wordmark and edited it to only contain the icon part then re inlined it. I didn't include the SVG asset since the full wordmark SVG asset isn't included currently. It would be nice to have those inline SVGs inserted automatically from the source SVG asset, but I think we should leave that to another issue. (just opened at #6493)
Yes it's the same as what API GL (static map api) uses. It feels right to use a smaller cutoff than the normal attribution as attribution is usually much wider to begin with. |
1e16e24
to
c887a94
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Andrew! This seems like a good change to me. I'm trying to track down someone on the product/legal side who can sign off on this and #6506. (Gotta make sure we're dotting our Is and crossing our Ts when it comes to branding and attribution.)
@@ -35,6 +36,10 @@ class LogoControl { | |||
|
|||
this._map.on('sourcedata', this._updateLogo); | |||
this._updateLogo(); | |||
|
|||
this._map.on('resize', this._updateCompact); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this binding in onRemove
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, fixed.
a.mapboxgl-ctrl-logo.mapboxgl-compact { | ||
width: 21px; | ||
height: 21px; | ||
background-image: svg-load('svg/mapboxgl-ctrl-logo-compact.svg'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding another SVG, could we hide the wordmark portion of the existing logo SVG by toggling a CSS class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea but since the logo is loaded as a background-image I don't think it's possible to control that through standard CSS, I think it would need to be an embedded SVG for that to work 🤔. Given it doesn't add much weight to the file size, I'd be tempted to leave it as is
Launch Checklist
closes collapsed Mapbox workmark on narrow maps #4282
same styling as the static maps api uses: