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

Advocacy site: updates to map & info box. #21806

Merged
merged 3 commits into from Apr 12, 2018
Merged

Conversation

breville
Copy link
Member

@breville breville commented Apr 11, 2018

advocacy.code.org

no state selected

screenshot 2018-04-11 17 14 18

wide page (narrow box)

screenshot 2018-04-11 17 14 31

narrow page (wide box)

screenshot 2018-04-11 17 14 51

code.org/promote

no state selected

screenshot 2018-04-11 17 16 17

wide page (narrow box)

screenshot 2018-04-11 17 16 28

narrow page (wide box)

screenshot 2018-04-11 17 16 42

Also some content changes to the four quarter slots on http://advocacy.code.org

Also some content changes to the rest of the site.
They accidentally got staged when reapplying a stash, but shouldn't have been committed.
@tanyaparker
Copy link
Contributor

Some thoughts

  • What do you think about making the state's name in the box teal too? So we don't have green/yet another color? Then you could change the /promote highlight color to teal to keep the highlight and title color the same on /promote too.
  • I don't think the x and check icons need to be so big. But we can try it. Why not.
  • The orange buttons look so tall. Do you think they need that much padding?
  • Feels like the 3 policies should be centered on the narrow page, wide box too.

@tanyaparker
Copy link
Contributor

Can you include screenshot of the box before a state is selected?

@@ -137,9 +137,6 @@
font-weight: bold;
text-align:center;
}
.fact {
font-size:15px;
Copy link
Contributor

Choose a reason for hiding this comment

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

teeny tiny nit: space between font-size and 15px

Copy link
Member Author

Choose a reason for hiding this comment

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

Fortunately that is being removed :)

Copy link
Contributor

@Erin007 Erin007 left a comment

Choose a reason for hiding this comment

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

Code changes look good.

@breville
Copy link
Member Author

  • colours updated per offline discussion.
  • :)
  • reduced them in size; big size came from earlier responsive explorations.
  • got them centering, though at the expense of fixing the width of that set of options. Unfortunately, CSS doesn't let you have the width of a parent and a child depend on each other, which is what I really wanted: a child that would maximise its width unless constrained by a parent, but also a parent that would constrain its width to that of its child so that it could be then centered. Fortunately, most viewers will probably see this at desktop width, in which case the hardcoded narrow width should be fine.

@tanyaparker
Copy link
Contributor

Screenshots LGTM - thanks!

@breville breville merged commit 55a3acf into staging Apr 12, 2018
@breville breville deleted the advocacy-map-changes branch April 12, 2018 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants