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

Update: Post publish buttons and alignment. #22390

Merged
merged 10 commits into from
Jun 25, 2020

Conversation

juanfra
Copy link
Member

@juanfra juanfra commented May 15, 2020

👋 Hello gutenfriends,

Description

I have implemented the changes proposed in this issue: #19175

The idea is to have a more clean and clear layout for the publish panel.

How has this been tested?

I have tested the changes by opening the page editor, and trying to publish a page. Checking that the buttons respond as they should, and that the changes represent the decisions taken in the issue. I have also ran tests, and update snapshots.

Tested:

  • Opened the page editor.
  • Added title and text block.
  • Hit "Publish" button.
  • Confirm that there's two buttons: "Publish" and "Cancel". Hitting "Publish" will indeed publish the page and open the confirmation panel, with the "Close" button. Hitting "Cancel" will close the panel.
  • I have also tested scheduling the page.

Screenshots

Types of changes

  • Updated block code to use the button without icon.
  • Updated the block code to act as the designs proposed in the issue.
  • Updated the code CSS to make that section look as the designs. Also removed the block code that is irrelevant due to these new changes.
  • Updated snapshots.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

juanfra added 2 commits May 15, 2020 16:18
Updating the post publish panel buttons to match the designs proposed on WordPress#19175
@paaljoachim
Copy link
Contributor

paaljoachim commented May 19, 2020

Great work @juanfra !
I checked the PR at gutenberg.run

Here is an animated gif of what it looks like:
Publish-flow-buttons

@folletto
Copy link
Contributor

Nice work.
Unfortunately the screenshot doesn't seem to load, can you update it?

From the gif it seems the buttons aren't aligned to the content tho:
Screenshot 2020-05-20 at 12 37 21

Can we make sure the left and right margins are aligned with the text below?

@juanfra
Copy link
Member Author

juanfra commented May 20, 2020

Thank you both for taking the time to look at this!

It seems that the gif I initially posted was too heavy for GitHub, sorry for that. I'm adding a new one now.

With regards to the alignment, great catch! that was the original padding for the panel header. Adjusted that to have the same as the other elements of the panel.

@folletto
Copy link
Contributor

Looks good to me now 👍

@paaljoachim
Copy link
Contributor

paaljoachim commented May 29, 2020

An update.
I mentioned the PR for Andrei @draganescu and he told me that the testing needs to be fixed in relation to the changes. @juanfra let us know if you need help with this part. This is Juanfra's first contribution to Gutenberg! Yayy! Which also means he might need some extra help.

@juanfra
Copy link
Member Author

juanfra commented May 29, 2020

Thanks @paaljoachim! Started checking the code of the project a couple weeks ago and I'm excited to contribute if possible.

Yes, at first I wasn't aware of the e2e tests so I only ran the basic npm test, updated snapshots and committed the proposed changes.

Then, once the PR was in place and I saw the results from travis I found out there were more. But, as the failing tests are part of the experiments folder, related to full site editing and multi entity saving I thought they weren't related to my changes (But I haven't had the time to check the entire codebase to see if the saving panel for the editor is being used elsewhere).

So Today I had a free hour and I tried to debug a bit further, but when I run e2e tests locally, there are a ton failing. So I guess I'm probably missing something. Any recommendations @draganescu?

@juanfra
Copy link
Member Author

juanfra commented May 29, 2020

Update: Used some time from my free hour to check this out, and it should be good to go now.

Thank you both!

@noisysocks noisysocks added [Type] Enhancement A suggestion for improvement. General Interface Parts of the UI which don't fall neatly under other labels. Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code labels Jun 22, 2020
@kirilzh
Copy link
Contributor

kirilzh commented Jun 22, 2020

Nice work. The new buttons look really good. 🎉

Approved your PR. @juanfra please resolve the any conflicts that have arisen.

@juanfra
Copy link
Member Author

juanfra commented Jun 22, 2020

thank you @kirilzh! Seems that we should wait on this #23370

@juanfra juanfra requested a review from kirilzh June 23, 2020 14:16
@paaljoachim
Copy link
Contributor

Hey @juanfra and @kirilzh #23370 has now been merged.
Lets get this PR merged as well..:)

@juanfra
Copy link
Member Author

juanfra commented Jun 25, 2020

Hi @paaljoachim!

Thanks for the follow-up. Yes, I merged the changes to this branch once it was merged. Everything is passing now, I'm waiting for a new approval + merge from @kirilzh

Thank you both :)

@kirilzh kirilzh merged commit e318033 into WordPress:master Jun 25, 2020
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jun 25, 2020
@github-actions
Copy link

Congratulations on your first merged pull request, @juanfra! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 25, 2020
@paaljoachim
Copy link
Contributor

Congratulations @juanfra !

@MichaelArestad
Copy link
Contributor

Nice job! It's a bit odd at specific screen sizes because the buttons stretch pretty wide. You can see that here:

image

This issue could be resolved by adding a max-width like this:

image

@juanfra juanfra deleted the fix/post-publish-panel-buttons branch June 25, 2020 15:21
@juanfra
Copy link
Member Author

juanfra commented Jun 25, 2020

Thanks @MichaelArestad,

Yeah, I agree, I could add that for viewports between $break-medium and $break-mobile. What number would you set as max-width? Not sure how long these words can get on different languages.

I'd be happy to push this fix :)

@MichaelArestad
Copy link
Contributor

Yeah, I agree, I could add that for viewports between $break-medium and $break-mobile. What number would you set as max-width? Not sure how long these words can get on different languages.

@juanfra Good question. I quickly picked an arbitrary number of 160px that looked about right and it's a multiple of 8. That might be good enough.

@juanfra
Copy link
Member Author

juanfra commented Jun 25, 2020

Thanks @MichaelArestad! Added a new PR with the changes #23487

sirreal added a commit to Automattic/wp-calypso that referenced this pull request Jul 27, 2020
fullofcaffeine pushed a commit to Automattic/wp-calypso that referenced this pull request Jul 27, 2020
* Change selector to match updated classes

See WordPress/gutenberg#22390
See WordPress/gutenberg@e318033

* Use nth-of-type (element has moved)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository General Interface Parts of the UI which don't fall neatly under other labels. Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants