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

Remove space between buttons v3 #5729

Merged
merged 6 commits into from
Aug 27, 2021
Merged

Remove space between buttons v3 #5729

merged 6 commits into from
Aug 27, 2021

Conversation

taineriley1
Copy link
Contributor

@taineriley1 taineriley1 commented Aug 20, 2021

Summary
Closes #5555
This PR attempts to implement the same changes as #5585 and #5685

I have moved the dropdowns from the right of the toolbar to the left.
I have also moved the 'delete' button to always be on the end.

Test plan
Tested locally in simple and editorial workflow.
Yarn test and cypress tests pass.

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • Code is formatted via running yarn format.
  • Tests are passing via running yarn test.
  • The status checks are successful (continuous integration). Those can be seen below.

A picture of a cute animal (not mandatory but encouraged)
image

@taineriley1 taineriley1 requested a review from a team August 20, 2021 03:22
@taineriley1
Copy link
Contributor Author

The failing build seems to be a network error.
@erezrokah can you please investigate?

@erezrokah erezrokah added the type: bug code to address defects in shipped code label Aug 20, 2021
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 @bradystroud!

I haven't tested it yet, but added a few comments on the code

@taineriley1
Copy link
Contributor Author

Thanks @erezrokah, removed the unnecessary fragments in the latest commit.

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 @taineriley1 and sorry for the late review.

Added a few comments.

Also, during my tests I noticed the Save notification hides the delete button.
I'll check if we can move it a bit down.

@taineriley1
Copy link
Contributor Author

Hi @erezrokah,

Thanks for the suggestions. I have implemented this in the latest commits and the code is much more readable now.

Thanks,
Taine

@erezrokah erezrokah merged commit 737573a into decaporg:master Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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