famanson / spring-roll-demo

Demo site for Project Spring Roll
1 stars 0 forks source link

Intro is now in footer with an overlayed message form #13

Closed famanson closed 10 years ago

famanson commented 10 years ago

@macduy please take a look

famanson commented 10 years ago

@macduy @mrbateuk Please also change the styling of the message form as you wish

macduy commented 10 years ago

@mrbateuk Could you please check out this branch and do a quick review? Thanks!

famanson commented 10 years ago

@macduy @mrbateuk This branch is working, in the sense that nothing is broken, only the design is not updated yet. Can you confirm that the code is ok, merge it first then all work on the design on post-theatre branch (where it is merged to)?

macduy commented 10 years ago

I'm happy with the styling now. However, the footer comes up too far when there are not enough posts - do a search and the "footer" appears in the middle of the page. We need to do somethign about that - maybe hide the footer when search is active?

famanson commented 10 years ago

or we can make it stick (fixed) when such situations happen?

macduy commented 10 years ago

Up to you. I'd say that is more work

Hai

On 28 May 2014 07:49, famanson notifications@github.com wrote:

or we can make it stick (fixed) when such situations happen?

— Reply to this email directly or view it on GitHubhttps://github.com/famanson/spring-roll-demo/pull/13#issuecomment-44371305 .

macduy commented 10 years ago

Plus, I think hiding is better because I want the search experience to be quite clean.

Hai

On 28 May 2014 07:51, Mac Duy, Hai macduy@gmail.com wrote:

Up to you. I'd say that is more work

Hai

On 28 May 2014 07:49, famanson notifications@github.com wrote:

or we can make it stick (fixed) when such situations happen?

— Reply to this email directly or view it on GitHubhttps://github.com/famanson/spring-roll-demo/pull/13#issuecomment-44371305 .

famanson commented 10 years ago

One could argue that we leave out too much white space.

I'll have a stab at it tonight

famanson commented 10 years ago

(of course you can merge atm)

macduy commented 10 years ago

if there isn't enough content, then there simply isn't enough content :)

famanson commented 10 years ago

@macduy I have updated search function. What it will do now:

Can you take a look to see if it is more intuitive to use and whether I broke anything?

macduy commented 10 years ago

:+1: Time to MERGE!

famanson commented 10 years ago

OMG