Closed koke closed 4 years ago
Could this one from the original PR be missed?
Ah yes, somehow I missed that comment. I added it to the list in the description
Current work in progress:
@iamthomasbishop I'm going to need some color names if possible for the text and some exact margins/positions for the subtitle.
Based on our conversation in slack (@koke and I), I'll drop some notes below.
In general, let's match colors that the native side is using where possible, which means:
Button/link colors: Like the apps, these should be blue-50
(light mode) and blue-30
(dark mode).
Title, subtitle colors: This is tricky because we'll be updating grays more broadly soon. For now, can we check both platforms and see if they're both the same colors, and match that? They should be using something like gray-90
for titles and gray-50
for subtitles, but let's double-check.
Honestly, I think the sizing/spacing looks good, even if not exact compared to the native versions. Just out of curiosity, for reference is there a way to "overlay" a semi-transparent example screen from the native side that has a similar layout, so that we can see the diff?
Also, would you be able to drop screenshots of what they look like on Android?
Just out of curiosity, for reference is there a way to "overlay" a semi-transparent example screen from the native side that has a similar layout, so that we can see the diff?
I can do that, but I believe subtitles are not a standard thing on iOS, so not sure what to compare to. The examples I'm seeing (WhatsApp, Telegram) all use the standard height and smaller text:
Without the subtitle, it was looking pretty close:
Also, would you be able to drop screenshots of what they look like on Android?
This is how it's looking right now:
On Android (because we're using what looks like 20px font-size?) it looks like the icon isn't centered. The close icon-button should be vertically centered with the text label.
For this I added a 1px top margin to the icon to adjust:
On Android, I think if we're mimicking the system App Bar style, the label should be set in Medium font-weight.
For this I'm using bold
. The previous attempt was setting font-weight: 600
, which works on iOS but not Android (I'm still using 600 to make Apply bolder on iOS)
For dark mode, not sure if I hit the right colors, but it's looking ok:
@koke Ah yes, dark mode! It’s looking pretty solid. I think you’re already using blue-30 for link buttons, and white for title so that’s all good. What colors are you using for the subtitle and separator? Both look fine, just want to make sure they’re consistent with other instances.
@koke And for the other examples you shared above, it’s all looking pretty good. One question: what size is the X
(close) icon on Android? It appears a tad small. IIRC, it should be a standard 24x24 icon.
What colors are you using for the subtitle and separator? Both look fine, just want to make sure they’re consistent with other instances.
For the subtitle I used $gray-60
(#50575e) for light mode and $light-opacity-200
for dark mode, which is 60% white. I don't think those belong to the same palette (the gray has a slight hue and the light is neutral), but I tried to reuse the existing variables that were closer to the guidelines.
@koke And for the other examples you shared above, it’s all looking pretty good. One question: what size is the
X
(close) icon on Android? It appears a tad small. IIRC, it should be a standard 24x24 icon.
The icon is set to 24x24, but that specific dashicon (no-alt
) seems to have a lot of padding.
For the subtitle I used $gray-60 (#50575e) for light mode and $light-opacity-200 for dark mode, which is 60% white. I don't think those belong to the same palette (the gray has a slight hue and the light is neutral), but I tried to reuse the existing variables that were closer to the guidelines.
Can we do use a neutral opacity gray on both? Perhaps something around 85-90% alpha w/ black on light mode, and the dark mode value is good. If not, this is looking pretty solid as is and I think we can do another pass to tweak when we do the upcoming colors audit/overhaul if need be.
The icon is set to 24x24, but that specific dashicon (no-alt) seems to have a lot of padding.
Ahh ok, that makes sense. Is there another Dashicons alternative that we can use? Maybe the material icon for close is available? (you can find it under Navigation > Close on the material docs). Or I think we have a Gridicons icon for the same thing, if that's available.
Can we do use a neutral opacity gray on both? Perhaps something around 85-90% alpha w/ black on light mode
@iamthomasbishop the only existing variable I see with a neutral gray is gray-900
(#1a1a1a), which looks to dark to me.
I'm fine with adding more variables, but I feel we should go with one of the existing ones, and plan to replace the existing colors with the new palette in a more coordinated effort.
Is there another Dashicons alternative that we can use? Maybe the material icon for close is available?
The material icon is looking better (not sure why it's not picking up the right color yet):
the only existing variable I see with a neutral gray is gray-900 (#1a1a1a), which looks to dark to me.
I think I might have misspoken. I was thinking 85-90% for the title color, but that’s not even what we were discussing 🤦♂️ For subtitle, it would be ~60%, which is pretty appropriate for secondary text. I think those neutrals are coming from / copied from Core, and I believe the 900
== 90%. So maybe 600, if there is one?
The material icon is looking better (not sure why it's not picking up the right color yet):
Def looks better. Can we compare that icon to the icon we’re using elsewhere in the apps (which I think is from Gridicons)? I believe it’s very close but doesn’t hurt to cross-reference .
Once we get the colors for icon and subtitle sorted, I’m all for shipping this.
Can we compare that icon to the icon we’re using elsewhere in the apps
That was hard to find, since we use a left arrow on Android for every modal I could find. In the Reader topics there's a X icon to remove tags that looks the same to me (although a few pixels larger)
For subtitle, it would be ~60%, which is pretty appropriate for secondary text
I ended up with this:
.title {
color: $dark-opacity-900;
}
.subtitle {
color: $dark-opacity-600;
}
Feedback originally posted by @iamthomasbishop in https://github.com/WordPress/gutenberg/pull/19106#issuecomment-572719302
[x] On Android (because we're using what looks like 20px font-size?) it looks like the icon isn't centered. The close icon-button should be vertically centered with the text label.
[x] On Android, I think if we're mimicking the system App Bar style, the label should be set in Medium font-weight.
[x] On iOS, it'd be great if we can use a text button that says
Close
or similar.[x] Are we going to add an
Apply
orUse
button to the right side of the header?[x] Is it possible to also add a sub-title on the header? If so we might want to bring the title font-size down to ~16px.
[x] I believe the icon and header title are both white in dark mode, but can we reference WPiOS to double-check? The background of the header should also match that of WPiOS, if possible – although I think we're using a dynamic system color so we might have to mimic the default color.
[x] One thing that needs tackling is, the modal is not adjusting to landscape on iOS, it stays in Portrait mode even if I rotate the device. (looks configurable)
Here are some exploratory mocks w/ different header variations: