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

New inputs.scss design #3121

Closed
skjnldsv opened this issue Jan 17, 2017 · 29 comments
Closed

New inputs.scss design #3121

skjnldsv opened this issue Jan 17, 2017 · 29 comments
Assignees
Labels

Comments

@skjnldsv
Copy link
Member

skjnldsv commented Jan 17, 2017

Since the inputs.scss file is a bit messy, I was wondering if it won't be better to rewrite everything.
Here's my results so far.

Side-by-side comparison: Codepen

Do you like the change? Do you find t more readable? What are the changes you would do?

@nextcloud/designers @jancborchardt

@Espina2
Copy link
Contributor

Espina2 commented Jan 17, 2017

You want opinions about code or visual?

@skjnldsv
Copy link
Member Author

@Espina2 visual

@MorrisJobke
Copy link
Member

I really like it a lot 👍

@Espina2
Copy link
Contributor

Espina2 commented Jan 17, 2017

I think the buttons can have a bit more padding and bit bigger.

@skjnldsv
Copy link
Member Author

@Espina2 What do you suggest? :)
Could be a good idea!

@MorrisJobke
Copy link
Member

And the time input is higher than all other inputs and therefore looks out of place.

@Espina2
Copy link
Contributor

Espina2 commented Jan 18, 2017

All the elements are the exact same thing as in the app? So I can have a quick overview how this are going to look?

@blizzz
Copy link
Member

blizzz commented Jan 18, 2017

I also really like it! One thing range seems kaput (on FF 50)? Nothing is visible.

@skjnldsv
Copy link
Member Author

@blizzz Noticed this too. Will fix.
@Espina2 I don't understand what you mean 😆

@Espina2
Copy link
Contributor

Espina2 commented Jan 18, 2017

Sorry my english sometimes is bad.

What I'm trying to ask is if are any visual changes to what we have now in nextcloud.

@skjnldsv
Copy link
Member Author

Yes, the codepen have two rows: the left one is the current nextcloud design, the right is my submission :)

@skjnldsv
Copy link
Member Author

@Espina2 @MorrisJobke @blizzz I updated the codepen. New feedback? 😄

@MorrisJobke
Copy link
Member

@Espina2 @MorrisJobke @blizzz I updated the codepen. New feedback? 😄

Awesome 🚀 🎉

@jancborchardt
Copy link
Member

jancborchardt commented Jan 18, 2017

@skjnldsv great stuff! Some points:

  • the hover and focus states shouldn’t modify the background. It’s especially confusing that it’s »greyed out« when you are in the field. Just leaving it white always (except when disabled) should be fine.
  • the active state with the blue border is also a bit strange since it only appears on mousedown for a splitsecond. Better: for hover, focus and active give the field that blue border, but leave the background white.
  • the button looks too much like an input field now, with the border. The current style with flat grey and border on hover has more affordance.
  • good on changing the select to be more styled like an input field than a button :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Jan 19, 2017

@jancborchardt Updated! :)
What do you think now? I tried something new to completely differentiate buttons/select from other inputs.

cc @MorrisJobke @Espina2 @ChristophWurst @nextcloud/designers

@eppfel
Copy link
Member

eppfel commented Jan 19, 2017

If we are on it, can we add :focus states for checkboxes and radio buttons?

@skjnldsv
Copy link
Member Author

@eppfel We can. I tried adding it to the codepen, but it's a pain adding svg to codepen as base64 ^^
Is there a possibility we could get rid of the svg and only use the checkmark?

Like we could set the border and stuff with css, would be easier to theme !

@skjnldsv
Copy link
Member Author

Okay, i figured it's pretty easy to do for the radio without svg at all. And for the checkbox, it won't be that hard. I will update the codependent (which the radio part is already updated) this afternoon.

It will be much more easier to theme and apply custom design with the incoming variables! 🎉

@blizzz
Copy link
Member

blizzz commented Jan 19, 2017

@Espina2 @MorrisJobke @blizzz I updated the codepen. New feedback? 😄

@skjnldsv same codepen link? Still have no range elements with FF 50 😿

@skjnldsv
Copy link
Member Author

@blizzz The range element seemed broken. Not coming from me: http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_type_range

Broken here too for me (ff 50.1.0). Seemed to be related to GTK+

@blizzz
Copy link
Member

blizzz commented Jan 20, 2017

Aw rly? On mobile currently and codepen works here with FF 50.1.0 mobile. Clearly, the Qt port should be revived ;)

@juliusknorr
Copy link
Member

@skjnldsv Great! Especially the buttons are a big improvement. 🎉

@eppfel
Copy link
Member

eppfel commented Jan 20, 2017

For a future PR of this, consider changes made in #2098

@skjnldsv
Copy link
Member Author

@eppfel, well the border on focus fit this purpose didn't it?

@skjnldsv
Copy link
Member Author

skjnldsv commented Jan 20, 2017

Aaaaaaaand! Here we go!
Absolutely NO IMAGES used on the new inputs. Including the radio and checkboxes.

@eppfel I added the border to the focus state. But I don't think @jancborchardt will like it. :)
http://codepen.io/skjnldsv/embed/QGLjMx/?height=1500&theme-id=dark&embed-version=2

@skjnldsv
Copy link
Member Author

Added white checkbox and radio! 🚀
capture d ecran_2017-01-20_18-50-10 capture d ecran_2017-01-20_18-50-24

@skjnldsv
Copy link
Member Author

Okay, this is starting to be a monologue, but I'm not gonna lie, I like it a lot! :D

capture d ecran_2017-01-20_19-05-54

Please more feedback!

@eppfel
Copy link
Member

eppfel commented Jan 20, 2017

@eppfel I added the border to the focus state. But I don't think @jancborchardt will like it. :)

We need focus highlighting and that's what he proposed earlier.

Better: for hover, focus and active give the field that blue border

I think for the checkboxes and radios this is still missing.

Overall I like it a lot, too 😍

But from the screenshot it still looks a bit odd. Maybe just because it is unfamiliar. I guess we need a PR to see it more in context.

@skjnldsv
Copy link
Member Author

Pr will follow tomorrow ;)

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

No branches or pull requests

7 participants