-
Notifications
You must be signed in to change notification settings - Fork 58
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
Block placeholder fixes #616
Conversation
I haven't tested yet, but from your screenshot, the placeholder doesn't look left-aligned with the title |
Also cc @Tug for i18n string changes |
Could we avoid saying "Welcome to Gutenberg", I don't think that would make sense to most of our users. Honestly I would stick with a neutral placeholder like "Add a title". |
@elibud – I think (iirc) the placeholder label text on the title does say One other thing that stands out to me is the margins of blocks on the initial canvas. The title feels too close to the navbar – maybe the title block is a little too tall, but it's hard to tell without seeing those boundaries. For now, can we add a slight bit of space at the top of the canvas (above the title block) so it feels a little more balanced? Maybe 16pt? |
@iamthomasbishop @SergioEstevao yes, looking at the code I can see we are keeping the "Add a Title" label, please ignore me 🤦♀️ |
@iamthomasbishop here is a new screenshot with the new margins and some background color to make it easier to figure margins. |
Keep in mind that those screens are for the example app, we should make sure that the margins look right on the WordPress apps as well |
@SergioEstevao Thanks for the reference. Spacing looks really wonky there – is the title vertically aligned top? That's probably why it looked like a lot of empty space 😄 In terms of spacing at the top of the canvas, I mocked this up just to get a visual reference, and 16pt/dp feels about right. It looks like this: Left: title focused. Right: static. I've also uploaded these to the Zeplin project for reference. |
@iamthomasbishop the component we use as the base for Aztec that is being used in the title always aligns the text on the top. I changed the dimensions of the component to make it more centred it now |
That looks a bit better already. Might still be worth adding 16pt of margin to the top of the canvas, but this at least looks cleaner. Side note, and maybe this is something that can be addressed in a separate issue – I noticed the color on all of the placeholder text labels is different from the preferred |
@iamthomasbishop updated the colors: |
@daniloercoli do you mind have a look in Android to see if things look ok after these changes? |
It does seem to work fine on Android! |
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.
Code looks good and the paragraph placeholders looks good and aligned with the title placeholder. 🎉
Tested in both WPiOS and WPAndroid ✅
I did notice something regarding the default block appender: It seems to be styled with a different color, and on iOS it has a different vertical alignment compared with the paragraph placeholder:
I'm not sure if the colors difference is intended and/or how important are those details right now.
Let me know what do you think and cc to @iamthomasbishop
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.
@SergioEstevao - The colors issue I mentioned was my bad! Colors are good ✅
After those new commits, the vertical alignment looks a lot better. ✨
Thanks for all of the quick iterations/feedback, @SergioEstevao and @etoledom! When ready, can you share screenshots of the current state on iOS/Android for design review? TY! |
Refs: #592, #549, #547
Updates the GB reference to have the placeholder fixes.
It also makes the title placeholder text o be consistent with the web and localized.
Related GB PR: WordPress/gutenberg#13945