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.

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

Closed
7 tasks done
koke opened this issue Jan 15, 2020 · 19 comments · Fixed by #1785
Closed
7 tasks done

Style improvements for template previews #1777

koke opened this issue Jan 15, 2020 · 19 comments · Fixed by #1785
Assignees

Comments

@koke
Copy link
Member

koke commented Jan 15, 2020

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 or Use 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:

image

@pinarol
Copy link
Contributor

pinarol commented Jan 20, 2020

Could this one from the original PR be missed?

  • 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)

@koke
Copy link
Member Author

koke commented Jan 20, 2020

Ah yes, somehow I missed that comment. I added it to the list in the description

@koke
Copy link
Member Author

koke commented Jan 20, 2020

Current work in progress:

Simulator Screen Shot - iPhone 8 - 2020-01-20 at 14 02 43

@iamthomasbishop I'm going to need some color names if possible for the text and some exact margins/positions for the subtitle.

@iamthomasbishop
Copy link
Contributor

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?

@koke
Copy link
Member Author

koke commented Jan 21, 2020

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:

IMG_38F1C449C140-1

Without the subtitle, it was looking pretty close:

navbar-diff

@koke
Copy link
Member Author

koke commented Jan 21, 2020

Also, would you be able to drop screenshots of what they look like on Android?

This is how it's looking right now:

spt-preview

@koke
Copy link
Member Author

koke commented Jan 21, 2020

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:

Screen Shot 2020-01-21 at 12 28 44

@koke
Copy link
Member Author

koke commented Jan 21, 2020

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)

@koke
Copy link
Member Author

koke commented Jan 21, 2020

For dark mode, not sure if I hit the right colors, but it's looking ok:

Simulator Screen Shot - iPhone 11 - 2020-01-21 at 17 53 47

@iamthomasbishop
Copy link
Contributor

@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.

@iamthomasbishop
Copy link
Contributor

@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.

@koke
Copy link
Member Author

koke commented Jan 21, 2020

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
Copy link
Member Author

koke commented Jan 21, 2020

@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.

@iamthomasbishop
Copy link
Contributor

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.

@koke
Copy link
Member Author

koke commented Jan 23, 2020

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.

Simulator Screen Shot - iPhone 11 - 2020-01-23 at 12 35 38

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.

@koke
Copy link
Member Author

koke commented Jan 23, 2020

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):

Screenshot_1579780369

@iamthomasbishop
Copy link
Contributor

iamthomasbishop commented Jan 23, 2020

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.

@koke
Copy link
Member Author

koke commented Jan 24, 2020

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)

android-close
Screen Shot 2020-01-24 at 09 40 47

@koke
Copy link
Member Author

koke commented Jan 24, 2020

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;
}

Simulator Screen Shot - iPhone 11 - 2020-01-24 at 09 47 19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants