Skip to content

for #1105, rearrange toolbar buttons #1109

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

for #1105, rearrange toolbar buttons #1109

wants to merge 1 commit into from

Conversation

raclim
Copy link
Collaborator

@raclim raclim commented Jun 21, 2019

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

catarak
catarak requested changes Jun 21, 2019
View reviewed changes
client/styles/components/_toolbar.scss
@@ -47,7 +48,11 @@
}
}
line-height: #{52 / $base-font-size}rem;
margin-left: auto;
// margin-left: auto;
Copy link
Member

Choose a reason for hiding this comment

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

can you remove this comment 😸

client/styles/components/_toolbar.scss
@@ -112,9 +122,14 @@
}
}

.toolbar__autorefresh {
// margin-right: #{300 / $base-font-size}rem;
Copy link
Member

Choose a reason for hiding this comment

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

also this comment ✨

client/styles/components/_toolbar.scss
@@ -153,12 +168,13 @@
height: #{18 / $base-font-size}rem;
margin-left: #{-4 / $base-font-size}rem;
& svg {
transform: scaleX(-1);
Copy link
Member

Choose a reason for hiding this comment

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

nice

@catarak
Copy link
Member

catarak commented Jun 21, 2019

overall this looks great! a few things that i noticed:

  • when you click on the sketch title to edit the name, the play/pause buttons jump to the right
  • the play/pause button positions depends on the length of the title
  • this is probably a pre-existing issue, but i think some of the icons are not centered within the buttons, and maybe now is the time for those buttons to get some tlc.

@catarak catarak closed this May 26, 2020
@catarak catarak reopened this May 26, 2020
@catarak catarak changed the base branch from master to develop August 26, 2020 19:06
@raclim raclim closed this Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catarak catarak catarak requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@raclim @catarak