-
Notifications
You must be signed in to change notification settings - Fork 57
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
Style improvements for template previews #1777
Comments
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:
Also, would you be able to drop screenshots of what they look like on Android? |
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: |
For this I'm using |
@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 |
For the subtitle I used |
The icon is set to 24x24, but that specific dashicon ( |
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.
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. |
@iamthomasbishop the only existing variable I see with a neutral gray is 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. |
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
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. |
Feedback originally posted by @iamthomasbishop in WordPress/gutenberg#19106 (comment)
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.
On Android, I think if we're mimicking the system App Bar style, the label should be set in Medium font-weight.
On iOS, it'd be great if we can use a text button that says
Close
or similar.Are we going to add an
Apply
orUse
button to the right side of the header?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.
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.
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:
The text was updated successfully, but these errors were encountered: