New colors set and css fixes #503

skjnldsv · 2016-05-05T10:04:49Z

This pr make the calendar app use the core colour generator to get an unified palette.

mention-bot · 2016-05-05T10:04:51Z

By analyzing the blame information on this pull request, we identified @raghunayyar, @jancborchardt and @tomneedham to be potential reviewers

georgehrke · 2016-05-05T10:05:36Z

js/app/directives/colorpickerDirective.js

+// expected lightness range: [0, 1]
+var hslToRgb = function(hue, saturation, lightness){
+  // based on algorithm from http://en.wikipedia.org/wiki/HSL_and_HSV#Converting_to_RGB
+  if( hue == undefined ){


please use tabs instead of spaces :)

Damn copy/paste :p

Wait a minute, the entire project uses space and not tabs, should I change the whole file @georgehrke ?

yes, please change the entire file

jancborchardt · 2016-05-05T11:33:42Z

I'm a bitnon the femce about the colors – the green looks too garish and the orange too flat. … but yeah they should come from the core generator.

Any way to shift that orange and green a bit to make it a tad nicer?

cc @georgehrke @raghunayyar @owncloud/designers check it out.

skjnldsv · 2016-05-05T14:20:01Z

We could also just add a colorwheel and remove the 9 colours ? :D

raghunayyar · 2016-05-10T09:59:00Z

@jancborchardt @skjnldsv just a suggestion, we can use HSLA colours and keep the saturation intact and change hue. The colors generated will probably will be on the same palette? What do you guys think? I think it works otherwise just the colours look too dull.

georgehrke · 2016-05-10T10:00:37Z

@raghunayyar We need to store RGB in the CalDAV property

skjnldsv · 2016-05-10T10:05:28Z

@raghunayyar we're already doing that in core.
We're using HSL and only changing the hue.

raghunayyar · 2016-05-10T10:51:03Z

@raghunayyar We need to store RGB in the CalDAV property

Can't we convert the HSLA to RGB in that case, when we are storing the colorcode?

skjnldsv · 2016-05-10T14:31:29Z

@raghunayyar have you read this pull request??

raghunayyar · 2016-05-10T15:15:00Z

@skjnldsv Lol, I am so sorry. Just reviewed the PR. I got quite emotional earlier seeing the colors being too light. Otherwise 👍

skjnldsv · 2016-05-10T15:20:01Z

Ahaha, no problem! :p
I was wondering if you didn't had the wrong pr! ^^

The colors being too light comes from the core generator then. But now that you said that, I maybe had forgotten to update the s&l values based on the luminosity. I need to take a look!

skjnldsv · 2016-05-10T15:51:21Z

Here we go. Rebased and ready to be merged.
I'm now using hardcoded values to generate the full hsl values, which I previously wasn't doing and therefore was ignoring the luminosity correction from core.

Now we're all set and ready to go!

If we want to shift the colors, we can also do that :)

skjnldsv · 2016-05-10T16:01:52Z

Shifting:
00:
10:
20:
30:

@jancborchardt

raghunayyar · 2016-05-10T19:15:18Z

👍

skjnldsv · 2016-05-10T19:17:15Z

Which one do you prefer? :)

georgehrke · 2016-05-10T19:18:00Z

I'd go with 00

aspdye · 2016-05-10T21:00:45Z

I am missing a "real" blue and some other natural, but i think this is nothing we can change here 😢
I'd also vote for 00.

MorrisJobke · 2016-05-10T21:09:49Z

I am missing a "real" blue and some other natural, but i think this is nothing we can change here

Why not. This sounds reasonable. :) @skjnldsv is this somehow possible to accomplish?

skjnldsv · 2016-05-11T08:20:53Z

You know what, here's what I'm proposing :)
We've got a better blue, and some good and various colours!

@aspdye

MorrisJobke · 2016-05-11T08:30:06Z

We've got a better blue, and some good and various colours!

I really like it :)

skjnldsv · 2016-05-11T09:23:35Z

And here's a gift:

georgehrke · 2016-05-11T10:31:47Z

Using chromium 52:

The colors are smaller (why?)
For some reason it shows a different icon for random, also clicking it does nothing

skjnldsv · 2016-05-11T12:50:00Z

Hum, I may have pushed the wrong folder! :p
I'm going to fix this when I get home ^^

The colors are smaller (why?)

Because there is a 10th element added to the list, so it has to take the full width.

This pr makes the spinner in front of the calendar name huge

See: #509, this is not related to this pr. I didn't changed anything related to the loader here.

EDIT: @georgehrke, I cheated, I did it from work! Could you check? :3

- Update css for colour palette - Fixed tabs - Use core generator with specific values

skjnldsv · 2016-05-11T18:13:50Z

Hum, should I edit js/public/app.js too?
Or is this file generated like the contacts app? I find it strange to have the same functions on two separate location :)

georgehrke · 2016-05-11T18:19:10Z

You have to build the app.js with grunt build inside js/

Please excuse my brevity and typos.
Sent from my mobile

On May 11, 2016, at 8:13 PM, John Molakvoæ [email protected] wrote:

Hum, should I edit js/public/app.js too?
Or is this file generated like the contacts app? I find it strange to have the same functions on two separate location :)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

skjnldsv · 2016-05-11T18:27:34Z

@georgehrke Peeeerfect! That's why this wasn't working for you :)
I was assuming the file would be compiled like owncloud/contacts.

Thank you!

georgehrke · 2016-05-11T18:29:50Z

How did you develop it without changing the app.js? ^^

Please excuse my brevity and typos.
Sent from my mobile

On May 11, 2016, at 8:27 PM, John Molakvoæ [email protected] wrote:

@georgehrke Peeeerfect! That's why this wasn't working for you :)
I was assuming the file would be compiled like owncloud/contacts.

Thank you!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

skjnldsv · 2016-05-12T02:48:33Z

@georgehrke I compiled in my head! :D
Just kidding, I manually edited the app.js because the makefile isn't compatible with my dev env.

skjnldsv · 2016-05-12T16:39:47Z

@raghunayyar, @jancborchardt, @tomneedham, @georgehrke, @MorrisJobke Review please?

georgehrke · 2016-05-12T16:59:00Z

👍

georgehrke · 2016-05-12T16:59:07Z

really like it! :)

skjnldsv · 2016-05-12T17:02:32Z

Thanks! :)

enoch85 · 2016-05-13T06:44:39Z

Nice work @skjnldsv 👍

georgehrke · 2016-05-20T20:20:21Z

rgbToHex is pretty broken 🙈
rgbToHex(0,100,0) gives me #0640

will fix it in #543

skjnldsv · 2016-05-20T20:29:28Z

@georgehrke is it due to a missing str_pad?
Don't recall the js function, but it may be due to the 0 converted to integers! :)

jancborchardt · 2016-06-08T10:20:52Z

Btw again @skjnldsv really good work! :)

skjnldsv · 2016-06-08T15:22:32Z

Thank you jan! :)

skjnldsv added 3 - to review design labels May 5, 2016

skjnldsv self-assigned this May 5, 2016

georgehrke reviewed May 5, 2016
View reviewed changes

georgehrke force-pushed the new-colors-set branch from a34a640 to deb5010 Compare May 9, 2016 11:04

skjnldsv added 2 - developing and removed 3 - to review labels May 10, 2016

skjnldsv force-pushed the new-colors-set branch from f5da6f6 to c33f40f Compare May 10, 2016 15:49

skjnldsv added 3 - to review and removed 2 - developing labels May 10, 2016

skjnldsv added 2 - developing and removed 3 - to review labels May 11, 2016

skjnldsv force-pushed the new-colors-set branch 3 times, most recently from 809f530 to 341f102 Compare May 11, 2016 13:20

New color generator

eea2721

- Update css for colour palette - Fixed tabs - Use core generator with specific values

skjnldsv force-pushed the new-colors-set branch from 341f102 to 17fa5c1 Compare May 11, 2016 18:00

skjnldsv force-pushed the new-colors-set branch from 4723473 to cb58ccf Compare May 11, 2016 18:19

New colours variations and random colour generator

63ccc7b

skjnldsv force-pushed the new-colors-set branch from cb58ccf to 63ccc7b Compare May 11, 2016 18:21

skjnldsv added 3 - to review and removed 2 - developing labels May 11, 2016

georgehrke merged commit 918df13 into master May 12, 2016

georgehrke deleted the new-colors-set branch May 12, 2016 16:59

New colors set and css fixes #503

New colors set and css fixes #503

Conversation

skjnldsv commented May 5, 2016 • edited Loading

mention-bot commented May 5, 2016

georgehrke May 5, 2016

Choose a reason for hiding this comment

skjnldsv May 5, 2016

Choose a reason for hiding this comment

skjnldsv May 5, 2016

Choose a reason for hiding this comment

georgehrke May 5, 2016

Choose a reason for hiding this comment

jancborchardt commented May 5, 2016

skjnldsv commented May 5, 2016

raghunayyar commented May 10, 2016

georgehrke commented May 10, 2016

skjnldsv commented May 10, 2016

raghunayyar commented May 10, 2016

skjnldsv commented May 10, 2016

raghunayyar commented May 10, 2016

skjnldsv commented May 10, 2016

skjnldsv commented May 10, 2016 • edited Loading

skjnldsv commented May 10, 2016 • edited Loading

raghunayyar commented May 10, 2016

skjnldsv commented May 10, 2016

georgehrke commented May 10, 2016

aspdye commented May 10, 2016 • edited Loading

MorrisJobke commented May 10, 2016

skjnldsv commented May 11, 2016 • edited Loading

MorrisJobke commented May 11, 2016

skjnldsv commented May 11, 2016

georgehrke commented May 11, 2016

skjnldsv commented May 11, 2016 • edited Loading

skjnldsv commented May 11, 2016

georgehrke commented May 11, 2016

skjnldsv commented May 11, 2016

georgehrke commented May 11, 2016

skjnldsv commented May 12, 2016

skjnldsv commented May 12, 2016

georgehrke commented May 12, 2016

georgehrke commented May 12, 2016

skjnldsv commented May 12, 2016

enoch85 commented May 13, 2016

georgehrke commented May 20, 2016

skjnldsv commented May 20, 2016

jancborchardt commented Jun 8, 2016

skjnldsv commented Jun 8, 2016

skjnldsv commented May 5, 2016 •

edited

Loading

skjnldsv commented May 10, 2016 •

edited

Loading

skjnldsv commented May 10, 2016 •

edited

Loading

aspdye commented May 10, 2016 •

edited

Loading

skjnldsv commented May 11, 2016 •

edited

Loading

skjnldsv commented May 11, 2016 •

edited

Loading