-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat: remove space between buttons #5585
Conversation
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.
Hi @bradystroud,
Looks like this doesn't play well with the deploy preview links, see
Hi @erezrokah As per our conversation, this is the button layout we have decided on. |
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.
Thanks for the follow up @bradystroud.
The preview link is still in the wrong place when not using editorial workflow:
You can reproduce by commenting this line:
https://github.com/netlify/netlify-cms/blob/3bd36776d6c02a0669df9a5b11f105efee2d2ca3/dev-test/config.yml#L6
packages/netlify-cms-core/src/components/Editor/EditorToolbar.js
Outdated
Show resolved
Hide resolved
packages/netlify-cms-core/src/components/Editor/EditorToolbar.js
Outdated
Show resolved
Hide resolved
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.
Hi @bradystroud, I've added a few more comments.
Please let me know what you think.
packages/netlify-cms-core/src/components/Editor/EditorToolbar.js
Outdated
Show resolved
Hide resolved
packages/netlify-cms-core/src/components/Editor/EditorToolbar.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Erez Rokah <[email protected]>
Co-authored-by: Erez Rokah <[email protected]>
…adystroud/netlify-cms into 5555-move-set-status-dropdown
Hi @bradystroud, that test is broken for me locally too. Due to the layout shift the 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? |
Something weird happened with git and I couldn't undo it. |
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
Figure: Open authoring view
Figure: Normal view