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