? Pending

User tests: Successful: Unsuccessful:

avatar C-Lodder
C-Lodder
22 Apr 2020

Pull Request for Issue #28463

Summary of Changes

Fix the styling issues present with the current custom-select.

  • Same SVG used for both LTR and RTL.
  • Simplified styling

Testing Instructions

  1. Run node build.js --compile-css from your terminal
  2. Go to the Module Manager in the backend and open any module
  3. Have a play around with the select dropdowns
  4. Open the inspector and replace dir="ltr" with dir="rtl" on the <html> node.

Screenshot

Screenshot_2020-04-22 Menus Edit Item - Joomla 4 - Administration

avatar C-Lodder C-Lodder - open - 22 Apr 2020
avatar C-Lodder C-Lodder - change - 22 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Apr 2020
Category Administration Templates (admin)
avatar C-Lodder C-Lodder - change - 22 Apr 2020
The description was changed
avatar C-Lodder C-Lodder - edited - 22 Apr 2020
avatar brianteeman
brianteeman - comment - 22 Apr 2020

Thank you

avatar infograf768
infograf768 - comment - 22 Apr 2020

Will test tomorrow.

avatar astridx
astridx - comment - 22 Apr 2020

Thank you. I have tested this PR and I like it and everything works as described.

ماژول ها  مسیر سایت   test   مدیریت
Modules  Breadcrumbs   test   Administration

However, I personally would prefer a green background behind a white arrow . At the moment there is a green background behind a black arrow.

Modules  Breadcrumbs   test   Administration(1)

Is this possible?

avatar C-Lodder
C-Lodder - comment - 22 Apr 2020

The styling can be argued about in someone elses PR/issue. Would like to keep this purely about the SVG issue

avatar infograf768
infograf768 - comment - 23 Apr 2020

I have mixed feelings here.

  1. It keeps using a svg (even if unique) instead of icons
  2. It looks like the svg color is defined by the variable $custom-select-indicator-color which therefore would have to be overridden if we want it to be white instead of $gray-900. Is that easy to do?
  3. The border color change is hardly readable for me.
avatar C-Lodder
C-Lodder - comment - 23 Apr 2020
  1. I'm not sure what you mean by this
  2. It's easy to override the variable
  3. Additional styling such as solid backgrounds (green/red) can be done by someone else. I'm not going to argue the toss about personal styling preferences in this PR. The objective here was to replace the multiple poorly constructed SVG's with something simple.

The only thing you need to be testing is:

  1. The SVG is visible
  2. The SVG is exactly the same in RTL without the need of an additional icon
  3. Setting a background-color for the <select> element is visible on the entire element
avatar infograf768
infograf768 - comment - 23 Apr 2020

It's easy to override the variable

For my info: can you give me an example of a scss that would override the variable when we need the svg to be white on a different background like Published/Unpublished (Greeen/Red)

avatar infograf768
infograf768 - comment - 23 Apr 2020

Here I get
Screen Shot 2020-04-23 at 09 55 00

avatar C-Lodder
C-Lodder - comment - 23 Apr 2020

@infograf768 Oh I see what you mean. If Atum was using PostCSS, this would easily be possible with scoped variables. With SCSS, I think you'd have to create a mixin.

Again, styling changes like the select BG colour are opinion based and are not part of this PR's scope. It can be argued about somewhere else.

avatar C-Lodder C-Lodder - change - 23 Apr 2020
The description was changed
avatar C-Lodder C-Lodder - edited - 23 Apr 2020
avatar infograf768
infograf768 - comment - 23 Apr 2020

Therefore, the only solution would be to create a new variable with a white filled svg and 2 new variables to use it in ltr/rtl?

avatar C-Lodder
C-Lodder - comment - 23 Apr 2020

That would basically just make this PR pointless. You would need to extended the changes done in this and use a mixin instead.

avatar dgrammatiko
dgrammatiko - comment - 23 Apr 2020

@C-Lodder the recommendation is that you shouldn't ever hardcode the fill in the svg so you can control it (class/inline css/whatever). So I guess you could remove the hardcoded bit from the svg part and add a fill: currentColor; in the customSelect class. That will cover 99.99% of the cases

avatar C-Lodder
C-Lodder - comment - 23 Apr 2020

@dgrammatiko You cannot use CSS's fill property on a background SVG.

avatar dgrammatiko
dgrammatiko - comment - 23 Apr 2020

Then use css custom properties ?

avatar C-Lodder
C-Lodder - comment - 23 Apr 2020

You can't do interpolation with a background url().

Using SCSS will require a mixin or a bunch of additional variables

avatar astridx
astridx - comment - 24 Apr 2020

You can't do interpolation with a background url().

Using SCSS will require a mixin or a bunch of additional variables

Am I correct? The main problem is that we don't use inline SVGs?

avatar C-Lodder
C-Lodder - comment - 24 Apr 2020

You can't use inline SVG's within a <select> element.

You've got 5 options that I can think of:

  • Move from SCSS to Post CSS to make use of locally scopped CSS variables
  • Use a SCSS mixin
  • Replace the SVG with a CSS caret
  • Go back to dirty SCSS
  • Don't use solid background (green/red)
avatar infograf768
infograf768 - comment - 24 Apr 2020

is it really impossible to use icons?
<span class="fas fa-angle-down" aria-hidden="true"></span>

avatar C-Lodder
C-Lodder - comment - 24 Apr 2020

@infograf768 <select> only allows <option> or <optgroup> children

avatar ciar4n
ciar4n - comment - 24 Apr 2020

If you wish to use triangles instead chevrons then this change should be done across the entire UI, something well beyond the intended scope of this PR.

avatar ciar4n
ciar4n - comment - 24 Apr 2020

I have tested this item successfully on 03816b9


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28755.

avatar ciar4n ciar4n - test_item - 24 Apr 2020 - Tested successfully
avatar astridx
astridx - comment - 24 Apr 2020

I was just about to see how everything could be switched to FontAwesome. (I just wanted to get some experience.) And now I believe that we should first standardize the HTML markup.

There are many different ways in which the select fields are inserted.

@C-Lodder When integrating with choices, your PR no longer shows an icon.

Modules  Breadcrumbs   test   Administration(2)

I don't understand why position is integrated with choices and status doesn't. Am I missing something? Both are single selects.

What do you think: shouldn't we first standardize the basis (HTML) and then build the CSS (SCSS) cleanly on it?

Regardless of whether we use (inline) SVG or FontAwesome.

avatar C-Lodder C-Lodder - change - 24 Apr 2020
Labels Added: ?
avatar C-Lodder
C-Lodder - comment - 24 Apr 2020

@astridx ChoicesJS styling is fixed.

The HTML is already standardised. It's just a simple <select> dropdown with options.

Probably just easier to remove the custom-select class from all elements and let the browser use it's own icon.

avatar astridx
astridx - comment - 24 Apr 2020

@C-Lodder
The HTML is already standardised. It's just a simple <select> dropdown with options.

Standardizes is a relative expression :)

Why

for position

<div class="control-group">
	<div class="control-label"><label id="jform_position-lbl" for="jform_position">
	Position</label>
</div>
	<div class="controls">
		<joomla-field-fancy-select class="" allow-custom="" search-placeholder="Type or select some options"><div class="choices" data-type="select-one" tabindex="0" role="combobox" aria-autocomplete="list" aria-haspopup="true" aria-expanded="false"><div class="choices__inner"><select id="jform_position" name="jform[position]" class="choices__input" tabindex="-1" data-choice="active" hidden=""><option value="breadcrumbs">breadcrumbs</option></select><div class="choices__list choices__list--single"><div class="choices__item choices__item--selectable" data-item="" data-id="1" data-value="breadcrumbs" data-custom-properties="null" aria-selected="true" data-deletable="">breadcrumbs<button type="button" class="choices__button_joomla" aria-label="Remove item: 'breadcrumbs'" data-button="">Remove item</button></div></div></div><div class="choices__list choices__list--dropdown" aria-expanded="false"><input type="text" class="choices__input choices__input--cloned" autocomplete="off" spellcheck="false" role="textbox" aria-autocomplete="list" aria-label=":: None ::" placeholder="Type or select some options"><div class="choices__list" role="listbox"><div class="choices__group " role="group" data-group="" data-id="841734803318" data-value=""><div class="choices__heading"></div></div><div id="choices--jform_position-item-choice-1" class="choices__item choices__item--choice choices__item--selectable is-highlighted" role="treeitem" data-choice="" data-id="1" data-value="" data-select-text="Press to select" data-choice-selectable="" aria-selected="true">:: None ::</div><div class="choices__group " role="group" data-group="" data-id="784749372895" data-value="Cassiopeia"><div class="choices__heading">Cassiopeia</div></div><div id="choices--jform_position-item-choice-2" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="2" data-value="banner" data-select-text="Press to select" data-choice-selectable="">Banner [banner]</div><div id="choices--jform_position-item-choice-3" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="3" data-value="bottom-a" data-select-text="Press to select" data-choice-selectable="">Bottom-a [bottom-a]</div><div id="choices--jform_position-item-choice-4" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="4" data-value="bottom-b" data-select-text="Press to select" data-choice-selectable="">Bottom-b [bottom-b]</div><div id="choices--jform_position-item-choice-5" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="5" data-value="breadcrumbs" data-select-text="Press to select" data-choice-selectable="">Breadcrumbs [breadcrumbs]</div><div id="choices--jform_position-item-choice-6" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="6" data-value="debug" data-select-text="Press to select" data-choice-selectable="">Debug [debug]</div><div id="choices--jform_position-item-choice-7" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="7" data-value="footer" data-select-text="Press to select" data-choice-selectable="">Footer [footer]</div><div id="choices--jform_position-item-choice-8" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="8" data-value="main-bottom" data-select-text="Press to select" data-choice-selectable="">Main-bottom [main-bottom]</div><div id="choices--jform_position-item-choice-9" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="9" data-value="main-top" data-select-text="Press to select" data-choice-selectable="">Main-top [main-top]</div><div id="choices--jform_position-item-choice-10" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="10" data-value="menu" data-select-text="Press to select" data-choice-selectable="">Menu [menu]</div><div id="choices--jform_position-item-choice-11" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="11" data-value="search" data-select-text="Press to select" data-choice-selectable="">Search [search]</div><div id="choices--jform_position-item-choice-12" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="12" data-value="sidebar-left" data-select-text="Press to select" data-choice-selectable="">Sidebar-left [sidebar-left]</div><div id="choices--jform_position-item-choice-13" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="13" data-value="sidebar-right" data-select-text="Press to select" data-choice-selectable="">Sidebar-right [sidebar-right]</div><div id="choices--jform_position-item-choice-14" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="14" data-value="top-a" data-select-text="Press to select" data-choice-selectable="">Top-a [top-a]</div><div id="choices--jform_position-item-choice-15" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="15" data-value="top-b" data-select-text="Press to select" data-choice-selectable="">Top-b [top-b]</div><div class="choices__group " role="group" data-group="" data-id="419122872835" data-value="Active Positions"><div class="choices__heading">Active Positions</div></div><div id="choices--jform_position-item-choice-16" class="choices__item choices__item--choice is-selected choices__item--selectable" role="treeitem" data-choice="" data-id="16" data-value="breadcrumbs" data-select-text="Press to select" data-choice-selectable="">breadcrumbs</div><div id="choices--jform_position-item-choice-17" class="choices__item choices__item--choice choices__item--selectable" role="treeitem" data-choice="" data-id="17" data-value="sidebar-right" data-select-text="Press to select" data-choice-selectable="">sidebar-right</div></div></div></div>
</joomla-field-fancy-select>
</div>
</div>

and

for published

<div class="control-group">
	<div class="control-label"><label id="jform_published-lbl" for="jform_published">
	Status</label>
</div>
	<div class="controls has-success">
		<select id="jform_published" name="jform[published]" class="custom-select custom-select-color-state custom-select-success valid form-control-success" size="1" aria-invalid="false">
	<option value="1" selected="selected">Published</option>
	<option value="0">Unpublished</option>
	<option value="-2">Trashed</option>
</select>
			</div>
</div>

Because of these differences (in one case joomla-field-fancy-select and in the other case not), it happened in my tests that the icon at position disappeared - just like yours in this PR. I expected that a select in which only one selection is possible would be the same everywhere.

avatar C-Lodder C-Lodder - change - 24 Apr 2020
Title
[4.0] Custom select styling improvements
[4.0] Custom select SVG improvements
avatar C-Lodder C-Lodder - edited - 24 Apr 2020
avatar brianteeman
brianteeman - comment - 24 Apr 2020

@astridx choicesJS is used where we need to select multiple AND/OR where we need to create on the fly eg a category or position

avatar astridx
astridx - comment - 24 Apr 2020

Thanks @brianteeman, I haven't seen that before. No I understand: Position is created with choicesJs because you can create position.

avatar C-Lodder C-Lodder - change - 25 Apr 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-04-25 14:00:50
Closed_By C-Lodder
avatar C-Lodder C-Lodder - close - 25 Apr 2020
avatar dgrammatiko
dgrammatiko - comment - 25 Apr 2020

@C-Lodder mate why did you close this one?
The idea to have a dependency on Font Awesome even for the simplest HTML elements is making me extremely sad...

avatar C-Lodder
C-Lodder - comment - 25 Apr 2020

Closing as it's being done here: #28787

avatar dgrammatiko
dgrammatiko - comment - 25 Apr 2020

@C-Lodder I saw it: hardcoding Font Awesome to all the select elements? What could possible go wrong there...

avatar Quy
Quy - comment - 19 Jul 2020

@C-Lodder #28787 is closed. Please reconsider opening this PR. Thank you!

Add a Comment

Login with GitHub to post a comment