Skip to content

Conversation

@thednp
Copy link
Contributor

@thednp thednp commented Mar 5, 2021

Re-commit for #30836, originally committed for #30288, now following up on #27212 and #29252 as suggested by @Fedik

The previous commit is obsolete since the above mentioned PRs have been merged, which means the previously suggested changes for PHP files are no longer necessary to improve the fancy-select layout.

This commit is much more inline with the current codebase and far better implementation.

Summary of Changes

As explained with the issue at hand, the choices.js provides support for a render limit of the <option>s and a search results limit, the first is much more important.

The source at build/media_source/system/js/fields/joomla-field-fancy-select.w-c.es6.js now can use data-max-render and data-max-results attributes to set searchResultLimit and renderChoiceLimit options for choices.js.

Testing Instructions

  • Open any extensionName.xml and add this field in any <fieldset>
    <field
      name="fancy-select"
      type="List"
      label="List Demo"
      default="5"
      data-max-render="5"
      data-max-results="15"
      layout="joomla.form.field.list-fancy-select">
      <option value="alpha0">Option Alpha</option>
      <option value="alpha1">Option Alpha 1</option>
      <option value="alpha2">Option Alpha 2</option>
      <option value="alpha3">Option Alpha 3</option>
      <option value="alpha4">Option Alpha 4</option>
      <option value="alpha5">Option Alpha 5</option>
      <option value="alpha6">Option Alpha 6</option>
      <option value="alpha7">Option Alpha 7</option>
      <option value="alpha8">Option Alpha 8</option>
      <option value="alpha9">Option Alpha 9</option>
      <option value="beta0">Option Beta</option>
      <option value="beta1">Option Beta 1</option>
      <option value="beta2">Option Beta 2</option>
      <option value="beta3">Option Beta 3</option>
      <option value="beta4">Option Beta 4</option>
      <option value="beta5">Option Beta 5</option>
      <option value="beta6">Option Beta 6</option>
      <option value="beta7">Option Beta 7</option>
      <option value="beta8">Option Beta 8</option>
      <option value="beta9">Option Beta 9</option>
      <option value="gama0">Option Gama</option>
      <option value="gama1">Option Gama 1</option>
      <option value="gama2">Option Gama 2</option>
      <option value="gama3">Option Gama 3</option>
      <option value="gama4">Option Gama 4</option>
      <option value="gama5">Option Gama 5</option>
      <option value="gama6">Option Gama 6</option>
      <option value="gama7">Option Gama 7</option>
      <option value="gama8">Option Gama 8</option>
      <option value="gama9">Option Gama 9</option>
      <option value="gama10">Option Gama 10</option>
    </field>
  • save and go to that extension config
  • open the field dropdown, there should be 5 options as configured;
  • type in 'option' in the search box of the field, you should get 15 results as configured.

Actual result BEFORE applying this Pull Request

Large lists are initialized extremely slow, also the initial max-results of 10 was not enough. Imagine more than 10 Roboto font family styles, or much more articles that contain a certain keyword / combination of keywords.

Expected result AFTER applying this Pull Request

Much faster initialization, no more browser notices on slow render, also more flexible configuration.

Documentation Changes Required

The following is a documentation draft on the use of the layouts/joomla/form/field/list-fancy-select.php layout with the new specific data-* attributes to be updated here.

Documentation Draft

The list form field comes with an alternate layout called "joomla.form.field.list-fancy-select" which enables additional functionality powered by Choices:

  • the ability to search though options;
  • the ability to limit the rendered options.

Example usage:

<field
    name="fieldc"
    type="list"
    label="FIELDC_LABEL"
    description="FIELDC_DESC"
    layout="joomla.form.field.list-fancy-select"
    data-max-render="-1"
    data-max-results="5"
    >
    <option value="0">JNO</option>
    <option value="1">Option 1</option>
    <option value="2">Option 2</option>
    <option value="3">Option 3</option>
    <option value="4">Option 4</option>
    <option value="5">Option 5</option>
    ....
    <option value="n">Option n</option>
</field>

The "joomla.form.field.list-fancy-select" layout uses two additional field properties:

  • the data-max-render="-1" property will limit the amount of visible options that are rendered on initialization as well as when expanded to the user; this option is useful when working with hundreds options, making them snappy and responsive; the default value is -1 which means unlimited;
  • the data-max-results="5" property will limit the amount of search results that are displayed to the user, which would suggest the user should use more specific keywords; the default value is 10.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Mar 5, 2021
@thednp thednp changed the title Update joomla-field-fancy-select.w-c.es6.js [4.0-dev] Add limit settings to fancy-select Mar 5, 2021
@thednp
Copy link
Contributor Author

thednp commented Mar 5, 2021

@Quy @Fedik this PR is now ready to test.

thednp added 2 commits March 5, 2021 17:11
`this.dataset` isn't working, but `this.select.dataset` is the right call.
@thednp
Copy link
Contributor Author

thednp commented Mar 5, 2021

@Fedik @Quy I've solved all issues and added a documentation draft. Please review.
Thanks

@Fedik
Copy link
Member

Fedik commented Mar 5, 2021

looks much better, I will try to test when will get some time

@Fedik
Copy link
Member

Fedik commented Mar 7, 2021

I have tested this item ✅ successfully on 6203cf6


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

@thednp
Copy link
Contributor Author

thednp commented Mar 7, 2021

@Fedik you will love this #32510, you might have to merge this #32315 to test this baby.

@thednp
Copy link
Contributor Author

thednp commented Mar 7, 2021

@Quy @richard67 RTC

@thednp
Copy link
Contributor Author

thednp commented Mar 10, 2021

@infograf768 this is a quick one you might wanna score a test :)

@ceford
Copy link
Contributor

ceford commented Mar 10, 2021

I put the test code in administrator/components/com_contacts/config.xml and it sort of works. The default list is 5 items. If I type in 'Option' I get 15 items. But if I type in '1' I get Option Alpha 1, Option Beta 1, and then the rest of the Betas, then Gama1 and Gama 10.

I have some lists with ~250 items and no way to guess what to type - so for me this fancy list seems counter-productive. Is there an example of where it might be really useful?


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

@thednp
Copy link
Contributor Author

thednp commented Mar 10, 2021

@ceford thanks for dropping by and test.

I have a list of 6.4k items from JSON, every time I open that list, the browser complains it took too long to draw (2-4 seconds). Also on the page with that huge list, the initialization takes 3-4 seconds more, so you can have an idea how much more of a difference is from 6.4k items to 20-30 on render (when showing the choices to the user or on initialization).

Perhaps in your case you could be more specific in the field description.

I put the test code in administrator/components/com_contacts/config.xml and it sort of works.

How the fancy-select works is in fact handled by choices.js, as we only configure it to some expected behavior. This PR is to expand a little bit of control over what choices renders to the user.

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

@Fedik there's something bothering me with the fancy select layout file itself. Compared to categoryedit field, the default ListField has nothing to do with the fancy select layout when it comes to handling of placeholder and search placeholder.

@ceford perhaps this is something on your mind as well?

Should I make adjustments here or commit a new PR?

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

ListField renders a native <select> field, by default.

<select> field does not have a placeholder,
there a "placeholder" is a first element in list

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

Then how does the categoryedit field set a search-placeholder? Using a custom layout, with parts that the default fancy-select layout doesn't have and should.

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

categoryedit use choices.js, and by default ListField does not use choices.js

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

categoryedit layout:

$placeholder = $this->escape(Text::_('JGLOBAL_TYPE_OR_SELECT_CATEGORY'));
$attr2 .= ' placeholder="' . $placeholder . '" ';
$attr2 .= ' search-placeholder="' . $placeholder . '" ';

fancy-select layout:

$attr2 = '';
$attr2 .= !empty($class) ? ' class="' . $class . '"' : '';
$attr2 .= ' placeholder="' . $this->escape($hint ?: Text::_('JGLOBAL_TYPE_OR_SELECT_SOME_OPTIONS')) . '" ';

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

@Fedik
placeholder-fancy

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

it does not exists for fancy-select layout

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

So perhaps we touch on what @ceford was concerned. In my books, as is now, with only our improvement with render limits and such, the fancy select layout is pretty much unusable.

This thing needs:

  • limit settings, added with this very PR
  • placeholder + option to allow non-value options (first option would hold the placeholder as you mentioned)
  • placeholder for search
  • placeholder for no result found

Without these, the fancy-select layout is unusable for custom components, extensions in general and @ceford is 100% right. Other devs will probably never make use of it without overriding the entire PHP and associated assets.

My question is: I want to contribute, should I commit to improve on these issues or it will just get pushed to J4.1 or later?

@brianteeman
Copy link
Contributor

FYI I am in the process of building a complex crm component and am using fancy-select extensively with no issues at all

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

FYI I am in the process of building a complex crm component and am using fancy-select extensively with no issues at all

Would you please share a sample fancy select field XML which uses placeholder, search-placeholder and no results message? Perhaps there's something I didn't see yet.

@brianteeman
Copy link
Contributor

tax

```
<field
		name="membershiptax"
		type="sql"
		label="COM_KEVUTZAH_FORM_LBL_FAMILY_MEMBERSHIPTAX"
		sql_select="id, membershiptax"
		sql_from="#__kevutzah_membership_tax"
		sql_where="state = 1"
		sql_order="ordering"
		key_field="id" 
		value_field="membershiptax"
		layout="joomla.form.field.list-fancy-select"
		multiple="true"/>

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

That doesn't answer my question..

@brianteeman
Copy link
Contributor

I dont see why not

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

no way to guess what to type

this should be in hint, like: "select an city", "select an book" etc
and it already possible

I do not see issue here.

But if you like you can make configurable almost any options of a Choices.js through data-.
Just be aware there will be an issue with translations.

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

But if you like you can make configurable almost any options of a Choices.js through data-.
Just be aware there will be an issue with translations.

Boy I would love to see that true. The attributes fancy-select Element used for fancy-select-layout are :

// Attributes to monitor
get allowCustom() { return this.hasAttribute('allow-custom'); }
get remoteSearch() { return this.hasAttribute('remote-search'); }
get url() { return this.getAttribute('url'); }
get termKey() { return this.getAttribute('term-key') || 'term'; }
get minTermLength() { return parseInt(this.getAttribute('min-term-length'), 10) || 1; }
get newItemPrefix() { return this.getAttribute('new-item-prefix') || ''; }
get placeholder() { return this.getAttribute('placeholder'); }
get searchPlaceholder() { return this.getAttribute('search-placeholder'); }
get value() { return this.choicesInstance.getValue(true); }
set value($val) { this.choicesInstance.setChoiceByValue($val); }

I don't see any dataset or data-* attribute here. The placeholder coming from hint field property only works with instances of multiple="true", which is very strange.

The choicesInstance is configured for fancy select only with these properties

// Init Choices
// eslint-disable-next-line no-undef
this.choicesInstance = new Choices(this.select, {
 placeholderValue: this.placeholder,
 searchPlaceholderValue: this.searchPlaceholder,
 removeItemButton: true, // THIS IS TRUE NO MATTER WHAT
 searchFloor: this.minTermLength,
 searchResultLimit: parseInt(this.select.dataset.maxResults, 10) || 10, // THIS IS CONFIGURABLE VIA data- PR #32591
 renderChoiceLimit: parseInt(this.select.dataset.maxRender, 10) || -1, // THIS IS CONFIGURABLE VIA data- PR #32591
 shouldSort: false, // THIS IS FALSE NO MATTER WHAT
 fuseOptions: {
  threshold: 0.3, // Strict search
 },
 noResultsText: Joomla.Text._('JGLOBAL_SELECT_NO_RESULTS_MATCH', 'No results found'),  // THIS CANNOT BE CONFIGURED
 noChoicesText: Joomla.Text._('JGLOBAL_SELECT_NO_RESULTS_MATCH', 'No results found'),  // THIS CANNOT BE CONFIGURED
 itemSelectText: Joomla.Text._('JGLOBAL_SELECT_PRESS_TO_SELECT', 'Press to select'),  // THIS CANNOT BE CONFIGURED

 // Redefine some classes
 classNames: {
  button: 'choices__button_joomla', // It is need because an original styling use unavailable Icon.svg file
 },
});

Those Joomla.Text right there are the cherry on top, which suggests devs can make language overrides in order to have something like "Select icon", "No Icon Found", etc.

IMO this code right here needs a complete rework. In my books this is unusable for my projects, I will have to override the layout and assets, use custom code just like categoryedit field, which will effectively invalidate all my work, including the previous attempts.

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

The placeholder coming from hint field property only works with instances of multiple="true", which is very strange.

Again "placeholder" does not exists for <select> tag, all we have is a kind of work around.
You can check Choices.js repository, for more info.

I don't see any dataset or data-* attribute here.

There is none, because it a custom element, and a needed attributes set in the layout.
However you have to use data- attribute if you want to have it configurable via XML, because it is not standard attributes for <select> input.
Btw, at time when it was coded there was not possible to add data- to the field XML.

In my books this is unusable for my projects

I have no idea what you doing there, but default configuration covers 99% needs. You can see @brianteeman example.

@thednp
Copy link
Contributor Author

thednp commented Mar 11, 2021

@Fedik

But if you like you can make configurable almost any options of a Choices.js through data-.
Just be aware there will be an issue with translations.

@Fedik
Copy link
Member

Fedik commented Mar 11, 2021

yes you can, I did not said that it already possible,
I meant you need to do some work for it 😉

@brianteeman
Copy link
Contributor

The discussion is interesting but its completely off topic to setting limits

@thednp
Copy link
Contributor Author

thednp commented Mar 12, 2021

Thank you Brian. Please score a test here and let's make it RTC.

@thednp
Copy link
Contributor Author

thednp commented Mar 15, 2021

Guys, is there a chance this PR gets into the next beta release please?

@joomdonation
Copy link
Contributor

@thednp If no-one test it today, I will test it on tomorrow.

@thednp
Copy link
Contributor Author

thednp commented Mar 15, 2021

@thednp If no-one test it today, I will test it on tomorrow.

Thanks buddy, please also see #32510

@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 6203cf6


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 16, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Mar 16, 2021
@rdeutz rdeutz merged commit d627b10 into joomla:4.0-dev Mar 18, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 18, 2021
@thednp thednp deleted the patch-3 branch March 18, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants