Date time picker position fix & design fixes by skjnldsv · Pull Request #7477 · nextcloud/server

skjnldsv · 2017-12-13T10:18:08Z

Position

Before	After

Design

Before	After

MorrisJobke · 2017-12-13T11:27:12Z

if the sidebar is scrolled (due to too small window) it adds the datepicker to the top:

skjnldsv · 2017-12-13T13:38:51Z

if the sidebar is scrolled (due to too small window) it adds the datepicker to the top:

Not sure this is fixable. The datepicker uses a fixed positioning and does not allow direction settings! 😕

jancborchardt · 2017-12-13T15:18:05Z

Looks good! Can we also make sure that the triangle pointer of the popover is aligned to be centered below the input field? This looks especially off in the calendar timepicker, where the popover is off to the right. cc @georgehrke

(Of course we have to make sure this also works on mobile.)

pixelipo

Whitespace on the left when 24-hour scheme used:

pixelipo · 2017-12-15T13:47:28Z

@skjnldsv I've noticed the ⬆️ issue while testing a non-absolute solution for AM/PM - Maybe you want to try with that as well:

 /* AM/PM fix */
- table.ui-timepicker tr th {
- position: absolute;
- margin-left: -24px;
- top: calc(50% + 5px);
+ table.ui-timepicker tr .ui-timepicker-hour-cell:first-child {
+ margin-left: 30px;
}
.ui-timepicker-table {
th {
font-weight: normal;
color: nc-lighten($color-main-text, 33%);
opacity: .8;
+ &.periods {
+ padding: 0;
+ width: 30px;
+ line-height: 30px;
+ }
}

 &.ui-timepicker-hours {
border-right: 1px solid $color-border;
- /* AM/PM fix */
- .ui-timepicker tr {
- position: relative;
- margin-left: 25px;
- }
}

codecov · 2017-12-20T09:38:04Z

Codecov Report

Merging #7477 into master will decrease coverage by 1.63%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #7477 +/- ##
============================================
- Coverage 52.81% 51.18% -1.64% - Complexity 23581 24948 +1367 
============================================
Files 1445 1605 +160 Lines 80357 94923 +14566 Branches 0 1376 +1376 ============================================
+ Hits 42440 48584 +6144 - Misses 37917 46339 +8422

Impacted Files	Coverage Δ	Complexity Δ
core/js/sharedialogshareelistview.js	`45.22% <0%> (ø)`	`0 <0> (?)`
.../tests/Unit/Collaboration/CommentersSorterTest.php	`25.55% <0%> (-66.45%)`	`6% <0%> (ø)`
apps/sharebymail/tests/SettingsTest.php	`52.17% <0%> (-47.83%)`	`3% <0%> (ø)`
lib/private/Security/RateLimiting/Limiter.php	`55.55% <0%> (-44.45%)`	`5% <0%> (ø)`
settings/Controller/EncryptionController.php	`54.71% <0%> (-38.84%)`	`8% <0%> (ø)`
settings/Controller/GroupsController.php	`64.61% <0%> (-35.39%)`	`9% <0%> (ø)`
...ps/comments/tests/Unit/AppInfo/ApplicationTest.php	`69.56% <0%> (-30.44%)`	`4% <0%> (ø)`
lib/private/AvatarManager.php	`71.42% <0%> (-28.58%)`	`4% <0%> (ø)`
apps/user_ldap/lib/Configuration.php	`42.02% <0%> (-27.86%)`	`87% <0%> (ø)`
apps/encryption/lib/Command/EnableMasterKey.php	`75% <0%> (-25%)`	`5% <0%> (ø)`
... and 403 more

skjnldsv · 2017-12-20T09:38:29Z

@pixelipo better?
How do you set 24h format btw?

pixelipo · 2017-12-20T10:14:02Z

@skjnldsv change the NC language (for example, Italian, German)

pixelipo · 2017-12-20T10:18:10Z

Still there, @skjnldsv :(

pixelipo

Two more things needed here:

Remove "Invalid date" text when no date is selected
Add a placeholder {{expirationDatePlaceholder}}

skjnldsv · 2017-12-21T15:02:35Z

@pixelipo if you want to add your fixes directly here, please do :)

rullzer · 2018-01-02T18:45:48Z

@pixelipo @skjnldsv any chance to get this done? Beta4 is coming

skjnldsv · 2018-01-03T08:36:46Z

@rullzer yes, it's going in!

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

MorrisJobke

Tested and works 👍

MorrisJobke · 2018-01-03T23:36:36Z

@pixelipo Could you again review this? Thanks

skjnldsv · 2018-01-04T18:34:03Z

Need the fixes that pixelipo requested. But the alignment can't be fixed properly.

rullzer

Let's do this 🐘

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv · 2018-01-05T07:26:48Z

@pixelipo All clear! Default expiry date is set to NOW+1day (can't expire the same day)
And default placeholder fixed :)

There is still the AM/PM issue, I'm scratching my head over this! 🤔

Please update review

MorrisJobke · 2018-01-05T09:54:15Z

I would go with this for now in the beta 4. The original issue was much more obvious and killed the proper selection of the expiry date. It is fixed in this PR and we should fix the other little paper cut regressions in followup PRs that will not make it into beta 4 but that should be fine.

MorrisJobke · 2018-01-05T09:54:29Z

I just tested again and it is fine as it is now.

pixelipo · 2018-01-05T09:55:55Z

Sorry I couldn't help you, guys - I'm sick and far away from my PC...

MorrisJobke · 2018-01-05T10:08:22Z

Sorry I couldn't help you, guys - I'm sick and far away from my PC...

Don't worry :) Was just to get stuff fixed, that is actually broken and fix the small issues later on. :)

MorrisJobke · 2018-01-05T10:08:40Z

And @pixelipo rest and get well soon :)

skjnldsv · 2018-01-05T11:23:33Z

@pixelipo never apologise for being sick! :)
Get well soon! 🎈

jancborchardt · 2018-01-05T16:38:45Z

Just one thing, was the default expiry date now+1day? Seems a bit short, and now+1week seems better. Otherwise a lot of links will expire by default without opening.

@pixelipo get well soon! :)

skjnldsv added 3. to review Waiting for reviews bug design Design, UI, UX, etc. regression labels Dec 13, 2017

skjnldsv added this to the Nextcloud 13 milestone Dec 13, 2017

skjnldsv self-assigned this Dec 13, 2017

skjnldsv requested review from MorrisJobke, jancborchardt, pixelipo and rullzer December 13, 2017 10:18

pixelipo requested changes Dec 15, 2017
View reviewed changes

skjnldsv force-pushed the date-time-picker-position-fix branch from 823f842 to c3d2e9bCompare December 20, 2017 10:32

This was referenced Dec 21, 2017
Fix custom share expiry date datepicker location #7595
Closed
Expiry date calendar for shares is misplaced #7007
Closed

skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 21, 2017

pixelipo previously requested changes Dec 21, 2017
View reviewed changes

MorrisJobke mentioned this pull request Jan 2, 2018
13.0.0 beta 4 #7664
Merged
30 tasks

Datepicker design fix
fe43330
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv added 2 commits January 3, 2018 14:02

Share datepicker fix
bcad100
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Fix am-pm
d190754
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv force-pushed the date-time-picker-position-fix branch from c3d2e9b to d190754Compare January 3, 2018 13:02

MorrisJobke approved these changes Jan 3, 2018
View reviewed changes

rullzer approved these changes Jan 5, 2018
View reviewed changes

Fixed default expire date and placeholder
6127d19
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv requested review from pixelipo and schiessle January 5, 2018 07:28

MorrisJobke merged commit 05bcdf9 into masterJan 5, 2018

MorrisJobke deleted the date-time-picker-position-fix branch January 5, 2018 09:54

Uh oh!

Uh oh!

Conversation

skjnldsv commented Dec 13, 2017

Position

Design

Uh oh!

MorrisJobke commented Dec 13, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skjnldsv commented Dec 13, 2017

Uh oh!

jancborchardt commented Dec 13, 2017

Uh oh!

pixelipo left a comment

Choose a reason for hiding this comment

Uh oh!

pixelipo commented Dec 15, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecovBot commented Dec 20, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skjnldsv commented Dec 20, 2017

Uh oh!

pixelipo commented Dec 20, 2017

Uh oh!

pixelipo commented Dec 20, 2017

Uh oh!

pixelipo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Dec 21, 2017

Uh oh!

rullzer commented Jan 2, 2018

Uh oh!

skjnldsv commented Jan 3, 2018

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Jan 3, 2018

Uh oh!

skjnldsv commented Jan 4, 2018

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Jan 5, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MorrisJobke commented Jan 5, 2018

Uh oh!

MorrisJobke commented Jan 5, 2018

Uh oh!

pixelipo commented Jan 5, 2018

Uh oh!

MorrisJobke commented Jan 5, 2018

Uh oh!

MorrisJobke commented Jan 5, 2018

Uh oh!

skjnldsv commented Jan 5, 2018

Uh oh!

jancborchardt commented Jan 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MorrisJobke commented Dec 13, 2017•
edited
Loading

pixelipo commented Dec 15, 2017•
edited
Loading

codecovBot commented Dec 20, 2017•
edited
Loading

pixelipo left a comment •
edited
Loading

skjnldsv commented Jan 5, 2018•
edited
Loading