Add colors datepicker, focus day when navigating #533

tbille · 2017-07-10T21:29:45Z

Proposition for this issue: #513

Add grey color to the week-ends and yellow color to the current day (today).

Bug fix: when changing month the selected day was the same as the current day. For example if the current day is 10th of July, when I changed month it was focusing the 10th of August.

This is fixed, now the first day of the month and the week is selected.

Tests: This pull-requests adds the missing tests on the DatepickerController.

mention-bot · 2017-07-10T21:29:47Z

@tbille, thanks for your PR! By analyzing the history of the files in this pull request, we identified @georgehrke, @raghunayyar and @tcitworld to be potential reviewers.

georgehrke · 2017-07-10T21:40:38Z

@tbille Thx a lot for this pull request! :)

Looks really cool! 🎉 :)

Add grey color to the week-ends and yellow color to the current day (today).

👍 As discussed in the issue

when changing month the selected day was the same as the current day.

This was actually the expected behavior for me, but let's open this up for discussion.
cc @jancborchardt @tcitworld

This pull-requests adds the missing tests on the DatepickerController.

Awesome! Much appreciated.

georgehrke · 2017-07-10T21:41:49Z

css/app/datepicker.css

 #datepicker table tbody tr.uib-weeks td:last-child button {
+	background-color: rgba(240, 240, 240, .9);


I didn't test this yet, but I think this will only work for US. What if you set your locale e.g. to german, where the week starts on monday? I guess Monday and Sunday will be shown a little darker

true, my work environment is set in US. This should be changed to work also with an European environment.

I'm not sure this approach will work for both locales.

Maybe we have to add another class to the date picker element, that says whether the week starts on Sunday or Monday.

ok it can be done in the function getDayClass of the datepickerController

done, now a class is added for everyday weekend days: weekendHilight

jancborchardt · 2017-07-12T14:02:22Z

This was actually the expected behavior for me, but let's open this up for discussion.

Yeah, I’d also say it’s expected behavior.

Great work @tbille! :)

jancborchardt · 2017-07-12T14:34:00Z

Just one point of feedback: In that datepicker there instead of giving the weekend full grey background, I would rather grey out the numbers. Otherwise it’s way too much visual load. :)

jancborchardt · 2017-07-12T14:41:02Z

css/app/datepicker.css

@@ -89,6 +89,14 @@
 	border-left: 0;
 }

+.todayHilight {


Also please use class naming like: .highlight-today and .highlight-weekend. :)

jancborchardt · 2017-07-12T14:58:25Z

I was working on a new datepicker design and since I made sure it works with the calendar datepicker too, how about we use the same design there?

See nextcloud/server#5691 (comment)

(circles should be round of course, it’s a rough mockup ;)
It would work exactly the same in the top left of the Calendar app navigation. Takes away all the extra lines and cruft, and makes the design a lot more modern, like the rest of Nextcloud. :)

tbille · 2017-07-15T13:08:24Z

@jancborchardt I applied your mockup on the datepicker, here how it looks like:

When not visible:

When visible:

Shall we apply the same style for the buttons month/weeks/day?

jancborchardt · 2017-07-15T18:26:25Z

Very nice! Looks good and I'd merge it. Cc @pixelipo since he just works on the date picker in core.

The week/month/today buttons should stay like this. :)

pixelipo · 2017-07-15T19:43:24Z

Looks great! I've only got two comments:

header should not be bold (same as core version)
today date would look better (and have better contrast) with a white font

jancborchardt · 2017-07-15T20:23:26Z

Right, agree with @pixelipo. Also, the selected date font should be bold. :)

tbille · 2017-07-16T10:46:57Z

@jancborchardt @pixelipo WDYT?

header should not be bold (same as core version)
today date would look better (and have better contrast) with a white font
the selected date font should be bold

jancborchardt

Great stuff! :)

jancborchardt · 2017-07-16T20:36:51Z

css/app/datepicker.css

@@ -89,27 +89,35 @@
 	border-left: 0;
 }

-#datepicker table tbody tr.uib-weeks td:last-child button {
-	border-right: none;
+.today-hilight {


Just one thing: Can you use the class exactly as I said in my previous comment:

.highlight-today

(Proper english and also the correct order, as we use .highlight-weekend too)

jancborchardt · 2017-07-16T20:37:12Z

css/app/datepicker.css

+	color: #c7c7c7;
+}
+
+.weekend-hilight button {


Same here, please call the class

.highlight-weekend

(Proper english and also the correct order, as we use .highlight-today too)

georgehrke · 2017-07-16T20:43:13Z

css/app/datepicker.css

 }

 #datepicker table tbody button {
 	max-width: 34px; /* Hack for Firefox for overflowing tables */
 }

 #datepicker table tbody button.active {
-	background-color: rgba(240, 240, 240, .9);
+	border-radius: 50%;
+	background-color: #0082c9;


@skjnldsv Can apps use scss too?

Yes, they can.

they should :) , but it should be in another issue.

It would be much easier to 'debug css' and to adapt the apps to the nextcloud themes

@georgehrke we migrated Deck to SCSS recently: nextcloud/deck#219

@georgehrke we migrated Deck to SCSS recently: nextcloud/deck#219

~~This looks wrong, because it doesn't use the scss engine from the Nextcloud server. Hence it won't update the CSS when the admin changes the theme~~

Nevermind

they should :) , but it should be in another issue.

Yes :)

I would vote to merge this now and fix the scss / primary color issue in a follow up PR :)

pixelipo · 2017-07-17T09:26:18Z

css/app/datepicker.css

@@ -89,27 +89,35 @@
 	border-left: 0;
 }

-#datepicker table tbody tr.uib-weeks td:last-child button {
-	border-right: none;
+.hilight-today {


As mentioned previously by @jancborchardt - this should be .highlight-today (spelling)

small mistake.
Done

pixelipo · 2017-07-17T09:27:27Z

css/app/datepicker.css

+	color: #c7c7c7;
+}
+
+.hilight-weekend button {


As mentioned previously by @jancborchardt - this should be .highlight-weekend (incorrect spelling)

Signed-off-by: Thomas Bille <contact@tbille.fr>

Very nice! Looks good and I'd merge it. (#533 (comment))

jancborchardt · 2017-07-30T22:42:04Z

Great work everyone, our new datepicker is awesome! :)

georgehrke requested changes Jul 10, 2017

View reviewed changes

georgehrke requested review from jancborchardt and tcitworld July 10, 2017 21:42

tbille force-pushed the add-colors-and-focus-day-datepicker branch from 4b14da6 to c3475b3 Compare July 10, 2017 22:08

georgehrke added the 3. to review Waiting for reviews label Jul 11, 2017

jancborchardt reviewed Jul 12, 2017

View reviewed changes

pixelipo mentioned this pull request Jul 13, 2017

Redesign jQuery UI datepicker nextcloud/server#5713

Merged

georgehrke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 13, 2017

tbille force-pushed the add-colors-and-focus-day-datepicker branch from c3475b3 to cd92ac9 Compare July 15, 2017 13:03

tbille force-pushed the add-colors-and-focus-day-datepicker branch from cd92ac9 to c86235e Compare July 16, 2017 10:42

jancborchardt approved these changes Jul 16, 2017

View reviewed changes

jancborchardt reviewed Jul 16, 2017

View reviewed changes

jancborchardt previously requested changes Jul 16, 2017

View reviewed changes

georgehrke reviewed Jul 16, 2017

View reviewed changes

tbille force-pushed the add-colors-and-focus-day-datepicker branch from c86235e to 3b13b7c Compare July 16, 2017 23:55

pixelipo requested changes Jul 17, 2017

View reviewed changes

Add colors datepicker, focus day when navigating

2164127

Signed-off-by: Thomas Bille <contact@tbille.fr>

tbille force-pushed the add-colors-and-focus-day-datepicker branch from 3b13b7c to 2164127 Compare July 17, 2017 10:01

pixelipo approved these changes Jul 17, 2017

View reviewed changes

georgehrke approved these changes Jul 17, 2017

View reviewed changes

georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 17, 2017

georgehrke merged commit ccb7aab into nextcloud:master Jul 18, 2017

tbille deleted the add-colors-and-focus-day-datepicker branch July 18, 2017 16:09

georgehrke mentioned this pull request Sep 12, 2017

Highlight current day in small calendar in left navigation bar too #513

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add colors datepicker, focus day when navigating #533

Add colors datepicker, focus day when navigating #533

tbille commented Jul 10, 2017

mention-bot commented Jul 10, 2017

georgehrke commented Jul 10, 2017

georgehrke Jul 10, 2017

tbille Jul 10, 2017

georgehrke Jul 10, 2017

tbille Jul 10, 2017

tbille Jul 10, 2017

jancborchardt commented Jul 12, 2017

jancborchardt commented Jul 12, 2017

jancborchardt Jul 12, 2017

jancborchardt commented Jul 12, 2017 •

edited

Loading

tbille commented Jul 15, 2017

jancborchardt commented Jul 15, 2017

pixelipo commented Jul 15, 2017

jancborchardt commented Jul 15, 2017

tbille commented Jul 16, 2017

jancborchardt left a comment

jancborchardt Jul 16, 2017

tbille Jul 16, 2017

jancborchardt Jul 16, 2017

tbille Jul 16, 2017

georgehrke Jul 16, 2017

raimund-schluessler Jul 16, 2017

tbille Jul 16, 2017

pixelipo Jul 17, 2017

georgehrke Jul 17, 2017 •

edited

Loading

georgehrke Jul 17, 2017

pixelipo Jul 17, 2017

tbille Jul 17, 2017

pixelipo Jul 17, 2017

tbille Jul 17, 2017

jancborchardt commented Jul 30, 2017

		#datepicker table tbody tr.uib-weeks td:last-child button {
		background-color: rgba(240, 240, 240, .9);

Add colors datepicker, focus day when navigating #533

Add colors datepicker, focus day when navigating #533

Conversation

tbille commented Jul 10, 2017

mention-bot commented Jul 10, 2017

georgehrke commented Jul 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jancborchardt commented Jul 12, 2017

jancborchardt commented Jul 12, 2017

Choose a reason for hiding this comment

jancborchardt commented Jul 12, 2017 • edited Loading

tbille commented Jul 15, 2017

jancborchardt commented Jul 15, 2017

pixelipo commented Jul 15, 2017

jancborchardt commented Jul 15, 2017

tbille commented Jul 16, 2017

jancborchardt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgehrke Jul 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jancborchardt commented Jul 30, 2017

jancborchardt commented Jul 12, 2017 •

edited

Loading

georgehrke Jul 17, 2017 •

edited

Loading