rubynz / kiwi-ruby-2017

The conference website for Kiwi Ruby 2017
https://2017-kiwi.ruby.nz
MIT License
2 stars 2 forks source link

Reduce dependencies #33

Closed ndorfin closed 7 years ago

ndorfin commented 7 years ago

In this PR:

Other possible improvements could include using 1 <nav> rather than 2 for the main menu.

mermop commented 7 years ago

I'll pass this one on to the Icelab team, using metaquery is their call - all seems reasonable to me but I'm not familiar with the benefits of using metaquery in the first place - @timriley @libbyberrie @tobydallder

timriley commented 7 years ago

I reckon if the same responsive layout changes have been achieved without metaquery, this is all good!

mermop commented 7 years ago

@raquelxmoss wrote the nav js - can you review the changes to that

mermop commented 7 years ago

Do you know a good casual image diff service? Since this changes the way responsive stuff is done, it'd be good to test it pretty thoroughly, but it can be tough to spot subtle problems when doing that manually. I've found percy.io pretty good but doesn't really work for this kind of thing - don't really want to set anything up permanently for this repo

mermop commented 7 years ago

Can you put some screenshots up? Changes if there's anything that's actually changed, or before/after if nothing should have changed - in particular the hamburger icon and the changes to the responsive styles

ndorfin commented 7 years ago

@mermop Will do - thanks! I'll find a nice SVG replacement for the Menu Icon, and then take some before and after screenies.

ndorfin commented 7 years ago

Phone Screenshots

Before After
before_closed after_closed
before_open after_open

Tablet screenshots

Before After
before_tablet after_tablet
mermop commented 7 years ago

Thanks for those! Only difference I can see is that the "menu" link is pink now for some reason?

ndorfin commented 7 years ago

@mermop Yup, I can happily swap it back to the grey. Made it red so that it matches other links - not sure why it was grey to begin with.

mermop commented 7 years ago

Let's go back to grey for it please :)

ndorfin commented 7 years ago

@mermop Done :)

mermop commented 7 years ago

Couple of conflicts from merging #29 - will aim to do some testing of this this weekend

ndorfin commented 7 years ago

@mermop Conflicts resolved

mermop commented 7 years ago

Noting that these changes make the mobile menu opening and closing animations less nice:

Original menu animation ![menu-original](https://user-images.githubusercontent.com/1615322/30246824-d59f832a-9658-11e7-8e69-f87c51384787.gif)
Changed menu animation ![menu-new](https://user-images.githubusercontent.com/1615322/30246831-ec268a9e-9658-11e7-8ee2-5878fc74081a.gif)
ndorfin commented 7 years ago

I'll start breaking this up into smaller PRs.