Date time picker position fix & design fixes

[skjnldsv]

Fix #7007

Position

Before	After

Design

Before	After

[MorrisJobke]

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

[skjnldsv]

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]

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]

@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]

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]

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

[pixelipo]

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

[pixelipo]

Still there, @skjnldsv :(

[pixelipo]

Two more things needed here:

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

[skjnldsv]

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

[rullzer]

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

[skjnldsv]

@rullzer yes, it's going in!

[MorrisJobke]

Tested and works 👍

[MorrisJobke]

@pixelipo Could you again review this? Thanks

[skjnldsv]

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

[rullzer]

Let's do this 🐘

[skjnldsv]

@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]

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]

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

[pixelipo]

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

[MorrisJobke]

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]

And @pixelipo rest and get well soon :)

[skjnldsv]

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

[jancborchardt]

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! :)