NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
10 Apr 2021

Pull Request for Issue #32997 redo of #16357 .

Summary of Changes

Use abetter script for marking HTML: https://markjs.io https://bundlephobia.com/result?p=mark.js@8.11.1

Testing Instructions

After pulling this PR and running npm ci:

Create an article with the word "create" and the word "admin" in the body text.
Ensure the System - Highlight plugin is enabled
View the article on the frontend
Then append the url you are on with ?highlight=WyJjcmVhdGUiLCJhZG1pbiJd

WyJjcmVhdGUiLCJhZG1pbiJd = base64encode of the array ["create","admin"]

Actual result BEFORE applying this Pull Request

No highlight

Expected result AFTER applying this Pull Request

Screenshot 2021-04-10 at 15 08 19

Documentation Changes Required

This should be 100% B/C but the idea is to deprecate as many HTMLHelpers as possible as they are really unfriendly for front end devs (nobody will ever create a Plugin just to adjust some HTML/JS/CSS and then have to maintain it)

@PhilETaylor can you give this a test?

avatar dgrammatiko dgrammatiko - open - 10 Apr 2021
avatar dgrammatiko dgrammatiko - change - 10 Apr 2021
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Apr 2021
Category ⇒ JavaScript Repository NPM Change Layout Libraries
avatar dgrammatiko dgrammatiko - change - 10 Apr 2021
Labels Added: NPM Resource Changed ?
avatar joomla-cms-bot joomla-cms-bot - change - 10 Apr 2021
Category JavaScript Repository NPM Change Layout Libraries ⇒ JavaScript Repository NPM Change Layout Libraries Front End Plugins
621ca42 10 Apr 2021 avatar dgrammatiko CS
dba3d44 10 Apr 2021 avatar dgrammatiko cs
avatar joomla-cms-bot joomla-cms-bot - change - 10 Apr 2021
Category JavaScript Repository NPM Change Layout Libraries Front End Plugins ⇒ JavaScript Repository NPM Change Front End com_finder Layout Libraries Plugins
avatar infograf768
infograf768 - comment - 11 Apr 2021

Works OK but imho the default bootstrap $mark-bg: #fcf8e3 !default; is too weak.
we have

mark, .mark {
    padding: 0.2em;
    background-color: #fcf8e3;
}

gives
Screen Shot 2021-04-11 at 09 46 04

I suggest overriding it with a stronger hue in .../templates/cassiopeia/scss/tools/variables/_variables.scss

for example: #fbeea8

which would give

Screen Shot 2021-04-11 at 09 54 58

avatar Fedik
Fedik - comment - 11 Apr 2021

It looks good,
but I have a couple notes :)

avatar Fedik
Fedik - comment - 11 Apr 2021

Now something more complicated ?

1) We do not need layout joomla.highlight.highlight.

You should not define ALL possible options in the layout.
The JS script should have a default options, and merge with User options, in this way it will be more efficient.
So, example, if your code have 'iframesTimeout'=> 5000, 'class' => 'js-highlight' then this should be in JS default, not PHP.
Then in PHP set only modified options:

$doc->addScriptOptions('highlight', [
 'highLight' => $terms,
])

Developer should set them only if he want diferent class, timeout:

$doc->addScriptOptions('highlight', [
 'highLight' => $terms,
 'class' => 'foo-bar',
])

In this way we save a loot of KB, and we do not need joomla.highlight.highlight.

2) For this script it better to allow to have a couple highlight on the page.

In addScriptOptions set array of options, instead of just options,
in this way we can have a couple highlight, for different terms:

$doc->addScriptOptions('highlight', [[ 'highLight' => $terms]], false);
$doc->addScriptOptions('highlight', [[ 'highLight' => $terms2]], false);

The JS could do some caching/checking to avoid the terms duplication, so we safe if someone have multiple call of same terms.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Apr 2021
Category JavaScript Repository NPM Change Layout Libraries Front End Plugins com_finder ⇒ JavaScript Repository NPM Change Front End com_finder Libraries Plugins
avatar dgrammatiko
dgrammatiko - comment - 11 Apr 2021

Now something more complicated

Should be ok now, fwiw I was copying fixing the old PR here without paying too much attention on my previous assumptions

avatar dgrammatiko
dgrammatiko - comment - 11 Apr 2021

@infograf768 as this PR is just replacing the highlighting script can you make a PR with the colour change once this is merged?

Edit: nevermind I added it here

avatar joomla-cms-bot joomla-cms-bot - change - 11 Apr 2021
Category JavaScript Repository NPM Change Libraries Front End Plugins com_finder ⇒ JavaScript Repository NPM Change Front End com_finder Libraries Plugins Templates (site)
avatar Fedik
Fedik - comment - 11 Apr 2021

I still would drop from php 'accuracy' => 'partially', 'diacritics' => true,
But it already better than before ?

Later I will try to test

00d450a 11 Apr 2021 avatar dgrammatiko DI
avatar Fedik
Fedik - comment - 17 Apr 2021

okay, I have tested, and it does not work ?
well, it partly work, only for ?highlight=WyJjcmVhdGUiLCJhZG1pbiJd
but not work for search result view

avatar Fedik Fedik - test_item - 17 Apr 2021 - Tested successfully
avatar Fedik
Fedik - comment - 17 Apr 2021

I have tested this item ✅ successfully on 86aaaf7


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

avatar particthistle particthistle - test_item - 17 Apr 2021 - Tested successfully
avatar particthistle
particthistle - comment - 17 Apr 2021

I have tested this item ✅ successfully on 86aaaf7

Created a highlight string to match content I had on my J4 installation and highlighting appeared as per expected result.


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

avatar Quy Quy - change - 17 Apr 2021
Status Pending ⇒ Ready to Commit
avatar Quy
Quy - comment - 17 Apr 2021

RTC


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

avatar rdeutz rdeutz - change - 19 Apr 2021
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2021-04-19 06:30:09
Closed_By ⇒ rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 19 Apr 2021
avatar rdeutz rdeutz - merge - 19 Apr 2021

Add a Comment

Login with GitHub to post a comment