famanson / spring-roll-demo

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

A message form potentially will be used in our feedback function #7

Closed famanson closed 10 years ago

famanson commented 10 years ago

@macduy Take a look when you have time?

cuctacuctac commented 10 years ago

Some of my thoughts:

famanson commented 10 years ago

@cuctacuctac seems like the wrong place for your comment here, Nhung :P

Anyhow, your first 2 points are in our backlog in the actual app, we just don't put them in the demo (especially the tagging system because I am sure that we'd have to refactor that big time when we move on to the real app)

@macduy can you please take a look at these colours suggestion please?

Very minor point but I'm not a big fan of the current colours used in Compose. Could we have Discard in the same shade of red as the heading for each post, and have New Ad in the darkest green that is used for the logo (on the 3 sushi roll lookalikes) please?

macduy commented 10 years ago

Mostly design:

macduy commented 10 years ago

"Prove that you're human! (Bottom-left checkbox)" - we shouldn't be telling users where the checkbox is, point the tooltip at it instead.

famanson commented 10 years ago

Yeah was about to do it but I realised the tooltip code is in compose. Should we make it a public thing?

On 24 May 2014 17:21, Mac Duy Hai notifications@github.com wrote:

"Prove that you're human! (Bottom-left checkbox)" - we shouldn't be telling users where the checkbox is, point the tooltip at it instead.

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

famanson commented 10 years ago

Get rid of the reds. Close should be grey so that it does not attract attention. Same for the bot thing. I know that the user needs to tick it but I don't find it desirable that we toot that in their face straight off the bat.

In the same place, a grey Send means it is disabled, would making Close grey as well be confusing?

macduy commented 10 years ago

Good point, I think the Close should just be a link, not a button.

macduy commented 10 years ago

Re tooltip: if it makes sense codewise, we should do it

macduy commented 10 years ago

Otherwise, I'm happy with the changes, feel free to merge/address comments in any order ;)

famanson commented 10 years ago

ok I'll see what I can do about Close before merging. Then we can do the tooltip refactoring after the merge

famanson commented 10 years ago

@macduy Alright, I made some of the styling changes. Can you take another pass then I'll merge

macduy commented 10 years ago

Mmm, nice! Alright, let's merge this. Honour's yours, @famanson

famanson commented 10 years ago

Merged, will :ship: to our demo site when I get all the php crap set up on my server :P

macduy commented 10 years ago

btw did you remove the "Show gridlines" checkbox in the public demo?

famanson commented 10 years ago

lol it is there. I'll disable it in prod

macduy commented 10 years ago

@mrbateuk will take a look at this and provide further design feedback