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

feat: remove space between buttons #5585

Closed
wants to merge 64 commits into from
Closed

feat: remove space between buttons #5585

wants to merge 64 commits into from

Conversation

bradystroud
Copy link
Contributor

Summary
closes #5555

I have combined the Toolbar subsections so all the buttons are together. I have also moved the delete button so it is always on the end.

Test plan
yarn test is passing

Screen Shot 2021-07-05 at 3 42 03 pm
Figure: Open authoring view

Screen Shot 2021-07-05 at 3 43 12 pm
Figure: Normal view

@bradystroud bradystroud requested a review from a team July 5, 2021 05:49
@bradystroud bradystroud marked this pull request as draft July 5, 2021 07:40
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jul 5, 2021
@bradystroud bradystroud marked this pull request as ready for review July 12, 2021 23:31
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradystroud
Copy link
Contributor Author

Hi @erezrokah

As per our conversation, this is the button layout we have decided on.

Screen Shot 2021-07-15 at 8 31 41 pm

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow up @bradystroud.

The preview link is still in the wrong place when not using editorial workflow:
image

You can reproduce by commenting this line:
https://github.com/netlify/netlify-cms/blob/3bd36776d6c02a0669df9a5b11f105efee2d2ca3/dev-test/config.yml#L6

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bradystroud, I've added a few more comments.

Please let me know what you think.

cypress/utils/steps.js Outdated Show resolved Hide resolved
@erezrokah
Copy link
Contributor

My changes a behaving as expected and the only test failing is the broken one (as per this comment #5595 (comment))

Hi @bradystroud, that test is broken for me locally too. Due to the layout shift the Entry Saved notification hides the Publish button in that tests. I fixed it via 2979363

Also, it seems this PR has many unrelated changes, for example https://github.com/netlify/netlify-cms/pull/5585/files#diff-61cc5e703786d80b2d74d323b857cd0cb7d3233c02482f479da2e3e20f450d20R6.

Can you please undo those?

@bradystroud bradystroud mentioned this pull request Aug 5, 2021
4 tasks
@bradystroud
Copy link
Contributor Author

Can you please undo those?

Something weird happened with git and I couldn't undo it.
I have created a new PR with the same changes - #5685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move 'Set status' dropdown and 'Save' button together
2 participants