Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve timepicker design, align to datepicker #7054

Merged
merged 4 commits into from
Nov 6, 2017
Merged

Improve timepicker design, align to datepicker #7054

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b01e979
Improve timepicker design, align to datepicker
jancborchardt Nov 3, 2017
153415c
Add vertical separator between hours and minutes
jancborchardt Nov 3, 2017
4e6d828
Bg variable for menu arrow
skjnldsv Nov 6, 2017
3e53844
Fix opacity of minutes in timepicker
jancborchardt Nov 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/css/header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ nav {
}

#navigation,
.ui-datepicker {
.ui-datepicker,
.ui-timepicker.ui-widget {
Copy link
Member

Choose a reason for hiding this comment

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

We have an issue with this line iirc. This needs to have its own properties I think! :/

Copy link
Member

Choose a reason for hiding this comment

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

Found it: #7007

Copy link
Contributor

Choose a reason for hiding this comment

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

@skjnldsv I still don't understand what's the issue there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, but this is separate from this pull request because it also applies to master. So let’s fix it in a separate PR, ok? :)

Copy link
Member

Choose a reason for hiding this comment

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

Okay! :)

position: relative;
left: -100%;
width: 160px;
Expand Down
70 changes: 70 additions & 0 deletions core/css/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,76 @@ code {
background: $color-main-background;
}


/* ---- jQuery UI timepicker ---- */
.ui-widget.ui-timepicker {
Copy link
Contributor

@pixelipo pixelipo Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@jancborchardt you don't want to just add classes to exiting datepicker declarations (lines 990-1066)?

edit: ok, if @skjnldsv wants to keep those separated, it's fine by me.

Copy link
Member Author

Choose a reason for hiding this comment

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

The structure is ever so slightly different that copying and adjusting seemed to make more sense. If you see a good way to combine it yes, however I think it’s better to separate it.

margin-top: 10px !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the !important required? Is there some CSS downstream affecting this? if so, it should either be moved before styles.scss or declarations in it should be dropped

Copy link
Member Author

Choose a reason for hiding this comment

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

Because in the header.scss we need to call it via this, to apply the popover styles:

 #navigation,
.ui-datepicker,
.ui-timepicker.ui-widget

Cause .ui-timepicker on its own is actually used inside the widget again (for the hours block and minutes block). And then we need to use the same identifier again in the styles.scss.

Copy link
Member Author

Choose a reason for hiding this comment

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

(It’s a bummer, I know, stupid structure they put there. ;) It looks quite funny if we only use .ui-timepicker in the header.scss ;D)

width: auto !important;
border-radius: $border-radius;

.ui-widget-content {
border: none !important;
}

.ui-state-default,
.ui-widget-content .ui-state-default,
.ui-widget-header .ui-state-default {
border: 1px solid transparent;
background: inherit;
}
.ui-widget-header {
padding: 7px;
font-size: 13px;
border: none;
background-color: $color-main-background;
color: $color-main-text;

.ui-timepicker-title {
line-height: 1;
font-weight: 300;
}
}
.ui-timepicker-table {
th {
font-weight: normal;
color: nc-lighten($color-main-text, 33%);
opacity: .8;
}
tr:hover {
background-color: inherit;
}
td {
> * {
border-radius: 50%;
text-align: center;
font-weight: normal;
color: $color-main-text;
padding: 8px 7px;
font-size: .9em;
line-height: 12px;
}

&.ui-timepicker-hour-cell a.ui-state-active,
&.ui-timepicker-minute-cell a.ui-state-active,
.ui-state-hover,
.ui-state-focus {
background-color: $color-primary;
color: $color-primary-text;
font-weight: bold;
}

&.ui-timepicker-minutes:not(.ui-state-hover) {
color: nc-lighten($color-main-text, 33%);
opacity: .8;
Copy link
Member

Choose a reason for hiding this comment

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

capture d ecran_2017-11-04_13-22-44

Copy link
Member Author

Choose a reason for hiding this comment

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

This is deliberate to separate the "more important" hours from the minutes, in addition to the separation line (it was there before).

Copy link
Member

Choose a reason for hiding this comment

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

The opacity?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the .8 opacity on the minutes.

Copy link
Member

Choose a reason for hiding this comment

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

It feels strange to have two blue colors then

}

&.ui-timepicker-hours {
border-right: 1px solid $color-border;
}
}
}
}

/* ---- DIALOGS ---- */

#oc-dialog-filepicker-content {
Expand Down