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.

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

Use SCSS for jquery-ui-fixes #6341

Merged
merged 3 commits into from
Sep 3, 2017
Merged

Use SCSS for jquery-ui-fixes #6341

merged 3 commits into from
Sep 3, 2017

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Sep 2, 2017

This PR contains several improvements for our jquery-ui-fixes:

  • Move to SCSS so we use theming color values
  • Apply tab style from files sidebar to ui-tabs (e.g. in user_ldap)
  • Move select style to the jquery-ui-fixes file and make those only apply to ui-autocomplete elements

Fixes #1090 #5348 #6178

bildschirmfoto von 2017-09-02 15-40-52

- Move to SCSS so we use theming color values
- Apply tab style from files sidebar to ui-tabs (e.g. in user_ldap)
- Move select style to the jquery-ui-fixes file and make those only apply to ui-autocomplete elements

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@mention-bot
Copy link

@juliushaertl, thanks for your PR! By analyzing the history of the files in this pull request, we identified @skjnldsv, @jancborchardt and @DeepDiver1975 to be potential reviewers.

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Very nice!! :)

@@ -8,29 +8,29 @@
font-family: "Lucida Grande", Arial, Verdana, sans-serif;
Copy link
Member

Choose a reason for hiding this comment

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

Drop it, because it is overwritten and outdated anyways. (same with the statement above)

@@ -8,4 +8,4 @@
@import 'fixes.scss';
@import 'multiselect.scss';
@import 'mobile.scss';
@import 'tooltip.scss';
@import 'tooltip.scss';
Copy link
Member

Choose a reason for hiding this comment

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

Unneeded change

@codecov
Copy link

codecov bot commented Sep 3, 2017

Codecov Report

Merging #6341 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master   #6341      +/-   ##
===========================================
- Coverage     53.31%   53.3%   -0.01%     
  Complexity    22497   22497              
===========================================
  Files          1403    1403              
  Lines         87079   87079              
  Branches       1327    1327              
===========================================
- Hits          46422   46421       -1     
- Misses        40657   40658       +1
Impacted Files Coverage Δ Complexity Δ
apps/files_trashbin/lib/Trashbin.php 72.28% <0%> (-0.25%) 136% <0%> (ø)

Signed-off-by: Julius Härtl <jus@bitgrid.net>
.ui-state-active {
border: none;
border-bottom: 1px solid $color-main-text;
color: $color-primary-text;
Copy link
Member

Choose a reason for hiding this comment

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

If this is primary text, then the background should be the accent color as well. Otherwise it should have the color-main-text. With a dark blue as theme it otherwise shows white background and white text.

bildschirmfoto 2017-09-03 um 16 07 02

@MorrisJobke MorrisJobke added this to the Nextcloud 13 milestone Sep 3, 2017
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

I fixed the font colouring:

bildschirmfoto 2017-09-03 um 18 32 22

@juliushaertl Please have a look if this is okay with you. Thanks

@juliushaertl
Copy link
Member Author

juliushaertl commented Sep 3, 2017

@MorrisJobke Makes sense 👍 Thanks for fixing.

@pixelipo
Copy link
Contributor

pixelipo commented Sep 4, 2017

Nice work, @juliushaertl

We should also move this stuff over to core/css/jquery-ui-fixes.scss :

server/core/css/styles.scss

Lines 990 to 1072 in ad5a298

/* ---- jQuery UI datepicker ---- */
.ui-widget.ui-datepicker {
margin-top: 10px;
padding: 4px 8px;
width: auto;
border-radius: 3px;
border: none;
.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-datepicker-title {
line-height: 1;
font-weight: 300;
}
.ui-icon {
opacity: .5;
&.ui-icon-circle-triangle-e {
background: url("../img/actions/arrow-right.svg") center center no-repeat;
}
&.ui-icon-circle-triangle-w {
background: url("../img/actions/arrow-left.svg") center center no-repeat;
}
}
.ui-state-hover .ui-icon {
opacity: 1;
}
}
.ui-datepicker-calendar {
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: 6px;
line-height: 12px;
}
&.ui-datepicker-today a:not(.ui-state-hover) {
background-color: nc-lighten($color-main-text, 86%);
}
&.ui-datepicker-current-day a.ui-state-active,
.ui-state-hover,
.ui-state-focus {
background-color: $color-primary;
color: $color-primary-text;
font-weight: bold;
}
&.ui-datepicker-week-end :not(.ui-state-hover),
.ui-priority-secondary:not(.ui-state-hover) {
color: nc-lighten($color-main-text, 33%);
opacity: .8;
}
}
}
}
.ui-datepicker-prev, .ui-datepicker-next {
border: nc-lighten($color-main-text, 86%);
background: $color-main-background;
}

@jancborchardt
Copy link
Member

Good stuff @juliushaertl! Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc. feature: ldap feature: theming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jQuery UI tabs should match the Nextcloud tab style
6 participants