Skip to content

emberjs.com styles#96

Merged
Gaurav0 merged 6 commits intoember-cli:masterfrom
miguelcobain:emberjs-theme
Aug 4, 2015
Merged

emberjs.com styles#96
Gaurav0 merged 6 commits intoember-cli:masterfrom
miguelcobain:emberjs-theme

Conversation

@miguelcobain
Copy link
Copy Markdown
Contributor

Opinions?

Styles need refactoring, IMO. I think they're a bit confusing. I tried to go along with the existing logic.
Depending on feedback I can refactor the styles further.

Preview:
Preview

@joostdevries
Copy link
Copy Markdown
Member

Nice. I do think we need to check with @tomdale or someone else at Tilde whether we're not infringing anything here ;-) Also, I believe some work on emberjs.com is underway, don't know what the impact on the styling will be.

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 29, 2015

Tomster is pretty well protected, but color scheme should be good. I'll ping @wifelette to just double check though.

@miguelcobain
Copy link
Copy Markdown
Contributor Author

@joostdevries I assumed there wasn't a problem since jsbin already has a similar theme. 👍

@wifelette
Copy link
Copy Markdown

👍 on the header, this fits with our overall strategy for where the brand bits can be used :)

The New Twiddle button doesn't look great, but I'll assume it's still being iterated on.

The new website stuff is definitely dragging, so I wouldn't wait on it. That said, once it comes out, we may want to update. Can cross that bridge when we get to it.

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Jul 29, 2015

👍 🚢

Comment thread app/styles/app.scss Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems somewhat odd to hotlink from emberjs.com directly. I think we should bring this .png down, into our own public folder.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rwjblue Definitely. I was just setting this up quickly to get feedback. Will fix this right away.

@miguelcobain
Copy link
Copy Markdown
Contributor Author

@wifelette Thank you for your input.

Regarding the New Twiddle button, that isn't exactly a button. It's an input where the user types the fiddle name. When you hover it its background transitions to a bright color.

Gif preview:
preview

Please let me know how I should handle this.

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 29, 2015

@miguelcobain - That bit of UX needs a bit of work (even before your changes). I think that can likely be tweaked laterish..

@wifelette
Copy link
Copy Markdown

Gotcha :)

I think it caught my eye for a few design reasons:

  • The way the copy can be cut off like that looks like a mistake.
  • The "I'm not editing right now" state looks odd. Unfinished. Also not 100% clear that it's editable.
  • The "I'm not editing right now" is also the same as the selected state on the JSBin this is modeled after; so we have the same users using a similar looking tool, but in one, this design means unselected, and in the other, it means click here to edit. Seems confusing.

There are surely better solutions out there, but potentially not ones worth waiting on. Unsure if there's anyone around with design chops (I'm assuming you're all devs, but correct me if I'm wrong!) though. (Any ideas, @sandraor?)

In the interim... some ideas:

  • Can the field expand to take up the full size of the copy you put into it, plus "New Twiddle" as the minimum width? If you name it something outrageously long we could have a max width and append ellipses to the end so you know there's more in there, if that's even necessary (versus a hard character limit)
  • Would the somewhat standard convention of a pencil icon beside the field make it more obvious?

This sort of thing:

2015-07-29_16-11-47

It's a different kind of example of course, but just making sure you know what I meant :p

@miguelcobain
Copy link
Copy Markdown
Contributor Author

@wifelette Your observations make a lot of sense.

An alternative would be to use the "Fork us" button style at http://emberjs.com/ with a pencil icon?

@wifelette
Copy link
Copy Markdown

That could be an option :)

In general, it might be a mistake to be thinking about a button and an editable window as the same thing, which it sounds like we're doing. We may want to create something entirely new and avoid all the confusion.

I chatted with @sandraor and she has some great ideas, stay tuned for her comments!

@sandraor
Copy link
Copy Markdown

I took a look at what's currently on http://ember-twiddle.com and I like that subtle approach to editing. I played around with the button styles from the http://emberjs.com/ header to come up with a few states:

Default
ember-twiddle-title-1

Hover (semi-transparent background)
ember-twiddle-title-2

Editing (darker background with box-shadow)
ember-twiddle-title-3

Since there's a hover state, I don't think it's essential to have an icon to indicate the edit feature. But it could work:
ember-twiddle-title-pencil

@wifelette
Copy link
Copy Markdown

MUCH improved here, and I still think we need the pencil icon to be sure it's clear. I say let's make it happen :D

Is there someone already set up who can do the implementation? Neither @sandraor nor I happen to already be set up with everything we need to run ember-cli on our machines, so these are just mockups so far.

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Jul 30, 2015

@wifelette @sandraor Feel free to use https://ember-twiddle-gaurav.herokuapp.com and Chrome / Firebug to directly edit the CSS until you like it and then and post it here. :)

@miguelcobain
Copy link
Copy Markdown
Contributor Author

I can do that. Looks really nice! Also agree on the pencil icon. If the user happens to never mouse over the title, he/she may never understand that it's editable.

@sandraor What are the transparency levels you used? Also, what values for box shadow?

@joostdevries
Copy link
Copy Markdown
Member

So much ❤️ for this :-)

@sandraor
Copy link
Copy Markdown

@Gaurav0 Thanks for the link!
@miguelcobain Here are the styles: https://gist.github.com/sandraor/7467980af9f1ac66fa47

Also, changing the pane header background colour to #faf4f1 (the background colour of emberjs.com) can lessen the contrast between the orange header and the content below:

ember-twiddle-panes

@miguelcobain
Copy link
Copy Markdown
Contributor Author

Any suggestions for including the pencil icon?

We can either include font-awesome, for example, or go with something like https://github.com/Kinvey/ember-cli-fontcustom

I use ember-cli-fontcustom myself and it works really great. However it would require contributors to install font custom on their pc's: https://github.com/FontCustom/fontcustom

Another option would be to use something like fontello or just simply include a png/svg file.

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Jul 31, 2015

I don't mind installing fontcustom on my PC, however for the sake of getting as many contributors as possible I would prefer to use ember-cli-font-awesome just because it downloads from bower and doesn't require another install.

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 31, 2015

I'm not opposed to any of these options, but in theory I would prefer vendoring the specific png/svg needed for the pencil icon (not sure if there are downsides there)...

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Jul 31, 2015

@rwjblue The advantage to using a font is that you can make it any color and size.

@acorncom
Copy link
Copy Markdown
Member

And using a font gives retina support out of the box for this icon (and potentially others down the road)

On Jul 31, 2015, at 12:17 PM, Gaurav Munjal notifications@github.com wrote:

@rwjblue The advantage to using a font is that you can make it any color and size.


Reply to this email directly or view it on GitHub.

@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jul 31, 2015

Derp, can't believe I got this all mixed up. Anyways, lets shoot for fontawesome (as opposed to requiring everyone to install fontcustom assuming I didn't misunderstand that too).

@miguelcobain
Copy link
Copy Markdown
Contributor Author

The downside of installing font awesome is that we're making everyone download a font with hundreds of glyphs just for one icon.

Fontcustom allows us to create our own icon font, with the icons we want.

Still, font-awesome is widely used. It may even be present in most of our caches. :)

@acorncom
Copy link
Copy Markdown
Member

acorncom commented Aug 3, 2015

Another option (that like Fontcustom allows creating a font with only a few glyphs) is https://icomoon.io I've used it in the past and been quite happy with it, but it might get to be a headache for Ember ...

There is this package that could ease things as well.
https://www.npmjs.com/package/icomoon-build

Personally, I think using FontAwesome from a CDN might be simplest though ... ;-)

@miguelcobain
Copy link
Copy Markdown
Contributor Author

I just noticed that bootstrap's glyphicons were being included from the start. I just went with that (it's also a font).

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Aug 4, 2015

Looks really good to me.

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Aug 4, 2015

@acorncom
Copy link
Copy Markdown
Member

acorncom commented Aug 4, 2015

In Safari 7.1 (OS 10.9.5), I'm seeing the "New Twiddle" button work beautifully:
safari

But in Chrome, the pencil icon just slightly chops off the end of the e:
chrome

Adjusting the CSS style width of the box to be 92px (from 89px) fixes the issue on my end ...

@miguelcobain
Copy link
Copy Markdown
Contributor Author

@acorncom That also happens to me on latest linux Chrome. That bug was present before these restylings, though. It should be related with ember-autoresize.

@miguelcobain
Copy link
Copy Markdown
Contributor Author

@acorncom Don't know how, but it looks fixed now!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

one last question: cant't the behaviour this provides also be done with CSS :focus?

@Gaurav0
Copy link
Copy Markdown
Contributor

Gaurav0 commented Aug 4, 2015

👍

1 similar comment
@joostdevries
Copy link
Copy Markdown
Member

👍

Gaurav0 added a commit that referenced this pull request Aug 4, 2015
@Gaurav0 Gaurav0 merged commit bcc359a into ember-cli:master Aug 4, 2015
@joostdevries
Copy link
Copy Markdown
Member

I get a different pencil on canary?

@miguelcobain
Copy link
Copy Markdown
Contributor Author

I see a different pencil too. Bootstrap assets are different?

@acorncom
Copy link
Copy Markdown
Member

@miguelcobain For future reference, looks like Fontello.com files (and config) were originally used for icons on the Ember website (emberjs/website@ff29f7c) Just found it when digging through to make a fix on the guides website ...

We should probably document that somewhere (for this repo and others) ;-)

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.

7 participants