Fix select2 dropdown #2133

skjnldsv · 2016-11-15T11:08:40Z

Fix #1932

@jancborchardt @MariusBluem @nickvergessen

mention-bot · 2016-11-15T11:08:42Z

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

jancborchardt · 2016-11-15T11:24:16Z

core/css/inputs.css

-	margin-right: 8px;
-	vertical-align: middle;
+.select2-container .select2-choice .select2-arrow b {
+	background: url("../img/actions/triangle-s.svg") no-repeat center;


Single quotes in CSS please ;)

jancborchardt · 2016-11-15T11:24:26Z

core/css/inputs.css

-	vertical-align: middle;
+.select2-container .select2-choice .select2-arrow b {
+	background: url("../img/actions/triangle-s.svg") no-repeat center;
+	opacity: 0.5;


No 0 before the dot please ;)

jancborchardt · 2016-11-15T11:24:42Z

core/css/inputs.css

-.select2-results .select2-selection-limit {
-	background: #fff !important;
-	padding: 12px !important;
+.select2-container .select2-choice:hover .select2-arrow b, .select2-container .select2-choice:focus .select2-arrow b, .select2-container .select2-choice:active .select2-arrow b {


Line break after the comma in CSS :)

jancborchardt · 2016-11-15T11:25:16Z

core/css/inputs.css

+#select2-drop .select2-search input {
+	width: calc(100% - 14px);
+	min-height: auto;
+	background: url("../img/actions/search.svg") no-repeat right center;


Single quotes

jancborchardt · 2016-11-15T11:25:50Z

cc @nextcloud/designers also for review and testing! :)

Fixed

skjnldsv · 2016-11-15T12:00:29Z

@jancborchardt done :)

jospoortvliet · 2016-11-15T13:45:22Z

nice!

btw @skjnldsv you should set yourself as 'public' in our project so you show up on https://nextcloud.com/contributors/ ;-)

skjnldsv · 2016-11-15T14:39:32Z

btw @skjnldsv you should set yourself as 'public' in our project so you show up on https://nextcloud.com/contributors/ ;-)

I like working in the darkness of nextcloud. ;)

jospoortvliet · 2016-11-15T21:01:26Z

He, well, you did make it to the nr 2 contributor in the last 7 days according to the nr of commits (yeah, flawed metric, but still) so 'dark' is a big word 👍 but it is, of course, your call, you have only 1 project on your profile page now - https://github.com/skjnldsv - we'd still be proud to show up, too, though 🤐

skjnldsv · 2016-11-16T05:56:32Z

I was joking! :)
It was set to private by default. It's public now! 🎉

jospoortvliet · 2016-11-16T08:56:53Z

Awesome, and welcome, you pushed us over 70 contributors!!! https://nextcloud.com/contributors/

juliusknorr · 2016-11-16T13:36:59Z

@skjnldsv .select2-searching and .select2-no-results should also have the same padding as .select2-result. Otherwise it is a bit to noisy while searching.

jancborchardt · 2016-11-16T13:38:21Z

And to add to @juliushaertl, the background of the »searching« and »No matches found« should be white :)

skjnldsv · 2016-11-16T18:16:58Z

Here you go :)

skjnldsv · 2016-11-17T06:20:52Z

@jancborchardt @jospoortvliet @juliushaertl @MariusBluem @nickvergessen
Review pls :)

nickvergessen · 2016-11-17T08:25:43Z

First commit is missing the sign-off

Works good and makes the field visible again 👍

schiessle · 2016-11-17T09:32:01Z

looks good, but please add the missing sign-off @skjnldsv. Thanks!

MorrisJobke · 2016-11-17T14:34:42Z

Doesn't work here in firefox and chrome:

skjnldsv · 2016-11-17T17:31:34Z

@MorrisJobke i'm using chromium and ff 50 and it works fine.

Please don't remove the review tag every time it doesn't work for you 😆
Did you cleared your cache?

@nickvergessen @schiessle sign-off added

juliusknorr · 2016-11-17T18:34:05Z

@MorrisJobke Works for me on ff and chrome.

@skjnldsv The padding and background looks good now, but it should also be applied to the searching string:

juliusknorr · 2016-11-17T18:35:31Z

core/css/inputs.css

+	padding: 0;
+}
+#select2-drop .select2-results .select2-result,
+#select2-drop .select2-results .select2-no-results {


adding #select2-drop .select2-results .select2-searching should do it. 😉

codecov-io · 2016-11-17T18:45:12Z

Current coverage is 57.74% (diff: 100%)

Merging #2133 into master will not change coverage

@@             master      #2133   diff @@
==========================================
  Files          1176       1176          
  Lines         70721      70721          
  Methods        7213       7213          
  Messages          0          0          
  Branches       1211       1211          
==========================================
  Hits          40837      40837          
  Misses        29884      29884          
  Partials          0          0

Powered by Codecov. Last update 332eaec...c0b376d

skjnldsv · 2016-11-17T20:10:51Z

@juliushaertl your desire is my command!

juliusknorr · 2016-11-18T10:34:58Z

@skjnldsv Looks good now. Another rebase to master and ready for review? 😉

jancborchardt · 2016-11-18T10:39:45Z

@skjnldsv @juliushaertl any chance to get this done today? It’ll be feature freeze :)

Fix #1932 + no result fix Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv · 2016-11-18T15:31:19Z

@jancborchardt Done! I applied your 3 last commits.

MorrisJobke · 2016-11-18T20:04:08Z

@MorrisJobke i'm using chromium and ff 50 and it works fine.

For me this is broken in both -.-

Did you cleared your cache?

Sure.

Please don't remove the review tag every time it doesn't work for you

I simply don't want broken behaviour - Both browsers run on macOS. On what OS have you tested it? Weird side fact: IE11 and Edge works just fine.

But nevertheless - let's merge this - maybe it's a super weird caching issue (even if I cleared it multiple times).

👍

skjnldsv · 2016-11-19T16:56:20Z

@MorrisJobke I didn't want to seems presumptuous, sorry if my message felt this way 🤗

Anyway, for the review tag, I was saying this because for a simple fix, I don't think switching back to develop, publishing a single small commit, then reswitching to review is a good idea (maybe i'm wrong 😄) since it only add more useless data to the pr flow 😝

jancborchardt · 2016-11-21T20:14:49Z

@skjnldsv thank you and sorry for all the edits I did on the inputs in the meantime ;)

jancborchardt · 2016-11-21T20:36:31Z

So @LukasReschke also just mentioned this looks a bit broken, and I can confirm:

Too much padding, and the width of the input is wrong (due to the calc(100%-14px)):

Dropdown overflows to the right:

@skjnldsv can you check this out?

skjnldsv · 2016-11-22T08:27:53Z

Hum, something else must have been added since everything was fine when i was testing this pr! 😕

skjnldsv · 2016-11-22T08:32:54Z

@jancborchardt I see you!! 😆
ff9d05e#diff-5d58c948797362ff794f7c4fb52b6527R262

jancborchardt · 2016-11-23T16:01:20Z

@skjnldsv thanks a lot! :)

skjnldsv added 3. to review Waiting for reviews design Design, UI, UX, etc. labels Nov 15, 2016

skjnldsv force-pushed the select2-fix branch from 1a33985 to 23a8f9c Compare November 15, 2016 11:10

jancborchardt previously requested changes Nov 15, 2016

View reviewed changes

skjnldsv force-pushed the select2-fix branch from 23a8f9c to 2ec7e79 Compare November 15, 2016 11:59

MorrisJobke mentioned this pull request Nov 16, 2016

Nextcloud 11 release review #2134

Closed

67 tasks

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 17, 2016

skjnldsv force-pushed the select2-fix branch from 81b30f7 to 197e964 Compare November 17, 2016 17:33

juliusknorr reviewed Nov 17, 2016

View reviewed changes

skjnldsv force-pushed the select2-fix branch from 197e964 to 65f98d5 Compare November 17, 2016 20:10

Fix select2 dropdown

c0b376d

Fix #1932 + no result fix Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv force-pushed the select2-fix branch from 65f98d5 to c0b376d Compare November 18, 2016 15:30

skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 18, 2016

MorrisJobke merged commit ece2205 into master Nov 18, 2016

MorrisJobke deleted the select2-fix branch November 18, 2016 20:04

MorrisJobke added this to the Nextcloud 11.0 milestone Nov 18, 2016

skjnldsv mentioned this pull request Nov 22, 2016

Select2 fix bis #2229

Merged

MorrisJobke mentioned this pull request Nov 22, 2016

Select2: Another broken dialogue in our current daily #2226

Closed

skjnldsv mentioned this pull request Nov 22, 2016

Too much spacing on select2 options #2244

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix select2 dropdown #2133

Fix select2 dropdown #2133

skjnldsv commented Nov 15, 2016

mention-bot commented Nov 15, 2016

jancborchardt Nov 15, 2016

jancborchardt Nov 15, 2016

jancborchardt Nov 15, 2016

jancborchardt Nov 15, 2016

jancborchardt commented Nov 15, 2016

skjnldsv commented Nov 15, 2016

jospoortvliet commented Nov 15, 2016

skjnldsv commented Nov 15, 2016

jospoortvliet commented Nov 15, 2016

skjnldsv commented Nov 16, 2016

jospoortvliet commented Nov 16, 2016

juliusknorr commented Nov 16, 2016

jancborchardt commented Nov 16, 2016

skjnldsv commented Nov 16, 2016

skjnldsv commented Nov 17, 2016

nickvergessen commented Nov 17, 2016

schiessle commented Nov 17, 2016 •

edited

Loading

MorrisJobke commented Nov 17, 2016

skjnldsv commented Nov 17, 2016 •

edited

Loading

juliusknorr commented Nov 17, 2016

juliusknorr Nov 17, 2016

codecov-io commented Nov 17, 2016 •

edited

Loading

skjnldsv commented Nov 17, 2016

juliusknorr commented Nov 18, 2016

jancborchardt commented Nov 18, 2016

skjnldsv commented Nov 18, 2016

MorrisJobke commented Nov 18, 2016

skjnldsv commented Nov 19, 2016

jancborchardt commented Nov 21, 2016

jancborchardt commented Nov 21, 2016 •

edited

Loading

skjnldsv commented Nov 22, 2016

skjnldsv commented Nov 22, 2016

jancborchardt commented Nov 23, 2016

Fix select2 dropdown #2133

Fix select2 dropdown #2133

Conversation

skjnldsv commented Nov 15, 2016

mention-bot commented Nov 15, 2016

jancborchardt Nov 15, 2016

Choose a reason for hiding this comment

jancborchardt Nov 15, 2016

Choose a reason for hiding this comment

jancborchardt Nov 15, 2016

Choose a reason for hiding this comment

jancborchardt Nov 15, 2016

Choose a reason for hiding this comment

jancborchardt commented Nov 15, 2016

skjnldsv commented Nov 15, 2016

jospoortvliet commented Nov 15, 2016

skjnldsv commented Nov 15, 2016

jospoortvliet commented Nov 15, 2016

skjnldsv commented Nov 16, 2016

jospoortvliet commented Nov 16, 2016

juliusknorr commented Nov 16, 2016

jancborchardt commented Nov 16, 2016

skjnldsv commented Nov 16, 2016

skjnldsv commented Nov 17, 2016

nickvergessen commented Nov 17, 2016

schiessle commented Nov 17, 2016 • edited Loading

MorrisJobke commented Nov 17, 2016

skjnldsv commented Nov 17, 2016 • edited Loading

juliusknorr commented Nov 17, 2016

juliusknorr Nov 17, 2016

Choose a reason for hiding this comment

codecov-io commented Nov 17, 2016 • edited Loading

Current coverage is 57.74% (diff: 100%)

skjnldsv commented Nov 17, 2016

juliusknorr commented Nov 18, 2016

jancborchardt commented Nov 18, 2016

skjnldsv commented Nov 18, 2016

MorrisJobke commented Nov 18, 2016

skjnldsv commented Nov 19, 2016

jancborchardt commented Nov 21, 2016

jancborchardt commented Nov 21, 2016 • edited Loading

skjnldsv commented Nov 22, 2016

skjnldsv commented Nov 22, 2016

jancborchardt commented Nov 23, 2016

schiessle commented Nov 17, 2016 •

edited

Loading

skjnldsv commented Nov 17, 2016 •

edited

Loading

codecov-io commented Nov 17, 2016 •

edited

Loading

jancborchardt commented Nov 21, 2016 •

edited

Loading