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 redundant rule. #10021

Merged
merged 2 commits into from
Sep 19, 2018
Merged

Remove redundant rule. #10021

merged 2 commits into from
Sep 19, 2018

Conversation

jasmussen
Copy link
Contributor

This addresses an issue reported in #9588 (comment).

It also polishes the comments a bit.

This addresses an issue reported in #9588 (comment).

It also polishes the comments a bit.
@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Sep 19, 2018
@jasmussen jasmussen added this to the 4.0 milestone Sep 19, 2018
@jasmussen jasmussen self-assigned this Sep 19, 2018
@jasmussen jasmussen requested a review from a team September 19, 2018 06:50
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Code seems fine except for that one comment that needs changing, so after that's fixed I say it's good to go!

@@ -11,12 +11,11 @@
left: 0;
right: 0;

// mobile edgecase for toolbar
// Stick the mobile toolbar way to the top, because the adminbar is not fixed either.
Copy link
Member

Choose a reason for hiding this comment

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

What does "way to the top" mean? I think it's a turn of phrase but I don't really understand 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

Also I think "adminbar" should be "admin bar".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catches, both of them.

@@ -52,14 +51,12 @@
}

.edit-post-header .components-button {
border-radius: $radius-round-rectangle;
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this is the actual redundant rule and the rest of the comment tweaks happened just because you were already tidying things up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. A good friend once suggested how these comments can look their best.

@jasmussen jasmussen merged commit 07368fc into master Sep 19, 2018
@jasmussen jasmussen deleted the fix/redundant-rule branch September 19, 2018 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants