Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

Improve size, positioning, and color of category icons in desktop dropdown (bug 1055701). #577

Closed
wants to merge 1 commit into from

Conversation

chuckharmston
Copy link
Contributor

r? @ngokevin @spasovski

This messes up the positioning of icons in the categories page, but that will get changed as part of the overhaul of bug 1055313, so I'm going to leave it for now.

catmenu

@chuckharmston
Copy link
Contributor Author

Changes to the SVG introduce space between each row of icons roughly equivalent to the spacing between icons.

@ngokevin
Copy link
Contributor

1055313 is about the navbar. I'm not sure category pages will get overhauled. If we know there's a regression on the cat page, shouldn't we fix it?

@@ -133,6 +133,9 @@
padding-left: 60px;
text-decoration: none;
width: 250px;
Copy link
Contributor

Choose a reason for hiding this comment

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

add linebreak

@chuckharmston
Copy link
Contributor Author

I know from a quick IRC chat that @pwalm will be including feedback about the icons on that page.

@spasovski
Copy link
Contributor

Yea we're breaking this everywhere else so it can be fixed in the dropdown. Maybe breaking is a strong word but this probably shrinks the intended size on the mobile pages and header titles. I'd rather wait on a visual review before merging this even though the logic seems solid.

@chuckharmston
Copy link
Contributor Author

Yeah, exactly

On Aug 19, 2014, at 1:38 PM, Kevin Ngo notifications@github.com wrote:

Oh do you mean the mobile categories page? Not the categories landing page?


Reply to this email directly or view it on GitHub.

@ngokevin
Copy link
Contributor

I'd feel more comfortable if we didn't regress what we have now. Any categories overhaul will take a day or two, and wouldn't want it messed up for those two days.

screen shot 2014-08-19 at 12 38 58 pm
screen shot 2014-08-19 at 12 38 45 pm
screen shot 2014-08-19 at 12 38 53 pm

@chuckharmston
Copy link
Contributor Author

Ugh, grepping for the SVG didn't hit those first two use cases. This is why sprites suck and we should get image fragments.

@chuckharmston
Copy link
Contributor Author

And this is why I should never touch Fireplace.

However, I still maintain that this bug get fixed at some point. It's an eyesore, and our sprite should have space between each icon if we're going to use it in several different places like this.

@ngokevin
Copy link
Contributor

Thanks for doing non-clipping the icons though. I pushed out the changes last Friday night with intention on iterating this week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants