Comments

matsbla created an issue. See original summary.

matsbla’s picture

Status: Active » Needs review
StatusFileSize
new1.48 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2800323-fix-styling-select-list.patch, failed testing.

The last submitted patch, 2: 2800323-fix-styling-select-list.patch, failed testing.

The last submitted patch, 2: 2800323-fix-styling-select-list.patch, failed testing.

matsbla’s picture

StatusFileSize
new0 bytes

Trying one more time.

matsbla’s picture

StatusFileSize
new1.48 KB
matsbla’s picture

Status: Needs work » Needs review
StatusFileSize
new1.48 KB

Status: Needs review » Needs work

The last submitted patch, 8: 2800323-fix-styling-select-list.patch, failed testing.

The last submitted patch, 8: 2800323-fix-styling-select-list.patch, failed testing.

The last submitted patch, 8: 2800323-fix-styling-select-list.patch, failed testing.

developmenticon’s picture

Status: Needs work » Needs review
StatusFileSize
new1.79 KB

Hi matsbla,
I am attaching a new patch to fix flag icon position and long country names. Please review it.
Thanks,
Himanshu

Status: Needs review » Needs work

The last submitted patch, 12: select_icons-Fix-styling-of-select-list-2800323.patch, failed testing.

The last submitted patch, 12: select_icons-Fix-styling-of-select-list-2800323.patch, failed testing.

The last submitted patch, 12: select_icons-Fix-styling-of-select-list-2800323.patch, failed testing.

vlad.dancer’s picture

+++ b/css/seven/selectmenu.css
@@ -51,9 +51,11 @@
+  font-size: 0.875rem;  /* 5 */

This looks very strange. Everywhere we are using 'px'. We should follow single measure unit for now.

Also please add an interdiff, because it's very difficult to do a review.

matsbla’s picture

+  font-size: 0.875rem;  /* 5 */

Not needed.

Please also add:

- font-weight: 600;
+ font-weight: 400;

Styling for RTL languages needs to be fixed.

developmenticon’s picture

Hi matsbla,
I am attaching a updated version of patch and also rtl issue fixed Please review it.
Thanks,
Himanshu

matsbla’s picture

StatusFileSize
new65.03 KB

Still not working for RTL languages.
Select icons RTL

+ width: auto !important;
Okay good, but maybe also:
+ min-width: 200px;

matsbla’s picture

StatusFileSize
new3.24 KB

Also needs
+ text-align: left;
for LTR and
+ text-align: right;
for RTL

If not the text in top is set to center:
Center text

developmenticon’s picture

Status: Needs work » Needs review
StatusFileSize
new2.86 KB

Updated patch with JS to recalculate country drop down for RTL-languages

Status: Needs review » Needs work

developmenticon’s picture

Status: Needs work » Needs review
StatusFileSize
new3.07 KB

Dropdown box margin removed for rtl, attached updated patch.

Status: Needs review » Needs work

vlad.dancer’s picture

Status: Needs work » Reviewed & tested by the community

Looks good for me. Tested on mobile and also with left aligned languages.
Thanks. Lets wait for matsbla response.

vlad.dancer’s picture

Status: Reviewed & tested by the community » Needs work

Oh, by the way, @developmenticon, replace tabs with whitespaces.

+++ b/js/ui.selecticons.js
@@ -63,6 +63,15 @@
+	    $('.selecticons').selecticons({
+		  position: {
+		    my: "right top",
+		    at: "right bottom",
+		    collision: "none"
+		  }
+		});

matsbla’s picture

@vlad.dancer:
Like this?

vlad.dancer’s picture

Status: Needs work » Reviewed & tested by the community

Yea, it is. @matsbla so are we ready to commit this changes?

Status: Reviewed & tested by the community » Needs work

vlad.dancer’s picture

Title: Fix styling of select list » Fix styling of select list (width, RTL, misc)
vlad.dancer’s picture

+++ b/css/seven/selectmenu.css
@@ -68,6 +69,32 @@
+/* Style flag position */
+.ui-widget.ui-selectmenu-button .ui-current-item-icon {
+  margin: -1px 0 0 0;
+  float: none;
+  vertical-align: middle;
+}
+.ui-selectmenu-menu .ui-menu-item {
+  overflow: hidden;
+  white-space: nowrap;
+  position: relative;
+  padding-left: 25px;
+}
+.ui-selectmenu-menu .ui-menu-item span.flag {
+  position: absolute;
+  left: 5px;
+  top: 6px;
+}
+[dir="rtl"] .ui-selectmenu-menu .ui-menu-item {
+  padding-right: 25px;
+  padding-left: 5px;
+}
+[dir="rtl"] .ui-selectmenu-menu .ui-menu-item span.flag {
+  left: auto;
+  right: 5px;
+}
+

Should be part of flags module, not of this one.

+++ b/js/ui.selecticons.js
@@ -63,6 +63,15 @@
+      if( $('.selecticons').css( "direction" ) == "rtl" ) {
+         $('.selecticons').selecticons({
+            position: {
+             my: "right top",
+            at: "right bottom",
+            collision: "none"
+          }
+      });
+   }

Various indent. Please, fix this.

vlad.dancer’s picture

Fixed indent, replaced tabs with spaces, removed .flag from css classes.

vlad.dancer’s picture

Status: Needs work » Needs review
matsbla’s picture

Status: Needs review » Needs work

Tested and works good, just a small detail:
+/* Style flag position */
"Style icon position"

vlad.dancer’s picture

Status: Needs work » Fixed
vlad.dancer’s picture

Status: Fixed » Closed (fixed)