From 2f319002644a003784b8431c615fccf47abe6687 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Tue, 1 Oct 2024 17:11:04 +0530 Subject: [PATCH 01/10] Fixed the Popover brief article issue --- www/js/lib/popovers.js | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index c4c829a45..63e99e8d1 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -36,7 +36,7 @@ import uiUtil from './uiUtil.js'; * @param {ZIMArchive} archive The archive from which to extract the lede * @returns {Promise} A Promise for the linked article's lede HTML including first main image URL if any */ -function getArticleLede (href, baseUrl, articleDocument, archive) { +function getArticleLede(href, baseUrl, articleDocument, archive) { const uriComponent = uiUtil.removeUrlParameters(href); const zimURL = uiUtil.deriveZimUrlFromRelativeUrl(uriComponent, baseUrl); console.debug('Previewing ' + zimURL); @@ -98,7 +98,7 @@ function getArticleLede (href, baseUrl, articleDocument, archive) { }; // Helper function to clean up the lede content -function cleanUpLedeContent (node) { +function cleanUpLedeContent(node) { // Remove all standalone style elements from the given DOM node, because their content is shown by innerText and textContent const styleElements = Array.from(node.querySelectorAll('style')); styleElements.forEach(style => { @@ -111,13 +111,18 @@ function cleanUpLedeContent (node) { // The reason we prefer innerText is that it strips out hidden text and unnecessary whitespace, which is not the case with textContent const innerText = para.innerText ? para.innerText : para.textContent; const text = innerText.trim(); - return !/^\s*$/.test(text) && text.length >= 50; + + // removing the para with less than 50 characters + // regex to check the paragraph if its too short or a brief description + const briefDescriptionRegex = /^.{1,100}$/; + + return !briefDescriptionRegex.test(text) && text.length >= 50; }); return parasWithContent; } // Helper function to concatenate paragraphs to fill the balloon -function fillBalloonString (paras, baseURL, pathPrefix) { +function fillBalloonString(paras, baseURL, pathPrefix) { let cumulativeCharCount = 0; let concatenatedText = ''; // Add enough paras to complete the word count @@ -149,7 +154,7 @@ function fillBalloonString (paras, baseURL, pathPrefix) { } // Helper function to get the first main image from the given node -function getImageHTMLFromNode (node, baseURL, pathPrefix) { +function getImageHTMLFromNode(node, baseURL, pathPrefix) { const images = node.querySelectorAll('img'); let firstImage = null; if (images) { @@ -176,7 +181,7 @@ function getImageHTMLFromNode (node, baseURL, pathPrefix) { * @param {Document} doc The document to which to attach the popover stylesheet * @param {Boolean} dark An optional parameter to adjust the background colour for dark themes (generally not needed for inversion-based themes) */ -function attachKiwixPopoverCss (doc, dark) { +function attachKiwixPopoverCss(doc, dark) { const colour = dark && !/invert/i.test(params.cssTheme) ? 'darkgray' : 'black'; const backgroundColour = dark && !/invert/i.test(params.cssTheme) ? '#111' : '#ebf4fb'; const borderColour = dark ? 'darkslategray' : 'skyblue'; @@ -237,8 +242,8 @@ function attachKiwixPopoverCss (doc, dark) { -webkit-touch-callout: none !important; } `, - // The id of the style element for easy manipulation - 'kiwixtooltipstylesheet'); + // The id of the style element for easy manipulation + 'kiwixtooltipstylesheet'); } /** @@ -250,14 +255,14 @@ function attachKiwixPopoverCss (doc, dark) { * @param {ZIMArchive} archive The archive from which the popover information is extracted * @returns {Promise
} A Promise for the attached popover div or undefined if the popover is not attached */ -function populateKiwixPopoverDiv (ev, link, state, dark, archive) { +function populateKiwixPopoverDiv(ev, link, state, dark, archive) { // Do not show popover if the user has initiated an article load (set in filterClickEvent) if (link.articleisloading || link.popoverisloading) return Promise.resolve(); const linkHref = link.getAttribute('href'); // Do not show popover if there is no href or with certain landing pages if (!linkHref || /^wikivoyage/i.test(archive.file.name) && - (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || - state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { + (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || + state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { return Promise.resolve(); } link.popoverisloading = true; @@ -326,7 +331,7 @@ function populateKiwixPopoverDiv (ev, link, state, dark, archive) { * @param {Event} event The event which has fired this popover action * @returns {Object} An object containing the popover div and the arrow span elements */ -function createNewKiwixPopoverCointainer (win, anchor, event) { +function createNewKiwixPopoverCointainer(win, anchor, event) { const div = document.createElement('div'); const linkHref = anchor.getAttribute('href'); const currentDocument = win.document; @@ -418,7 +423,7 @@ function createNewKiwixPopoverCointainer (win, anchor, event) { * @param {Element} popover The containing element of the popover (div) * @param {Document} doc The doucment on which to operate */ -function addEventListenersToPopoverIcons (anchor, popover, doc) { +function addEventListenersToPopoverIcons(anchor, popover, doc) { const breakout = function (e) { // Adding the newcontainer property to the anchor will be cauught by the filterClickEvent function and will open in new tab anchor.newcontainer = true; @@ -438,7 +443,7 @@ function addEventListenersToPopoverIcons (anchor, popover, doc) { * Remove any preview popover DIVs found in the given document * @param {Document} doc The document from which to remove any popovers */ -function removeKiwixPopoverDivs (doc) { +function removeKiwixPopoverDivs(doc) { const divs = doc.getElementsByClassName('kiwixtooltip'); // Timeout is set to allow for a delay before removing popovers - so user can hover the popover itself to prevent it from closing, // or so that links and buttons in the popover can be clicked @@ -466,7 +471,7 @@ function removeKiwixPopoverDivs (doc) { * Closes the specified popover div, with fadeout effect, and removes it from the DOM * @param {Element} div The div to close */ -function closePopover (div) { +function closePopover(div) { div.style.opacity = '0'; // Timeout allows the animation to complete before removing the div setTimeout(function () { From da805538431d23bbc82a00e8a1a2851d0db1ec53 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Tue, 1 Oct 2024 17:31:18 +0530 Subject: [PATCH 02/10] Fixed the Popover issue --- www/js/lib/popovers.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 63e99e8d1..b1ec740a4 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -36,7 +36,7 @@ import uiUtil from './uiUtil.js'; * @param {ZIMArchive} archive The archive from which to extract the lede * @returns {Promise} A Promise for the linked article's lede HTML including first main image URL if any */ -function getArticleLede(href, baseUrl, articleDocument, archive) { +function getArticleLede (href, baseUrl, articleDocument, archive) { const uriComponent = uiUtil.removeUrlParameters(href); const zimURL = uiUtil.deriveZimUrlFromRelativeUrl(uriComponent, baseUrl); console.debug('Previewing ' + zimURL); @@ -98,7 +98,7 @@ function getArticleLede(href, baseUrl, articleDocument, archive) { }; // Helper function to clean up the lede content -function cleanUpLedeContent(node) { +function cleanUpLedeContent (node) { // Remove all standalone style elements from the given DOM node, because their content is shown by innerText and textContent const styleElements = Array.from(node.querySelectorAll('style')); styleElements.forEach(style => { @@ -122,7 +122,7 @@ function cleanUpLedeContent(node) { } // Helper function to concatenate paragraphs to fill the balloon -function fillBalloonString(paras, baseURL, pathPrefix) { +function fillBalloonString (paras, baseURL, pathPrefix) { let cumulativeCharCount = 0; let concatenatedText = ''; // Add enough paras to complete the word count @@ -154,7 +154,7 @@ function fillBalloonString(paras, baseURL, pathPrefix) { } // Helper function to get the first main image from the given node -function getImageHTMLFromNode(node, baseURL, pathPrefix) { +function getImageHTMLFromNode (node, baseURL, pathPrefix) { const images = node.querySelectorAll('img'); let firstImage = null; if (images) { @@ -181,7 +181,7 @@ function getImageHTMLFromNode(node, baseURL, pathPrefix) { * @param {Document} doc The document to which to attach the popover stylesheet * @param {Boolean} dark An optional parameter to adjust the background colour for dark themes (generally not needed for inversion-based themes) */ -function attachKiwixPopoverCss(doc, dark) { +function attachKiwixPopoverCss (doc, dark) { const colour = dark && !/invert/i.test(params.cssTheme) ? 'darkgray' : 'black'; const backgroundColour = dark && !/invert/i.test(params.cssTheme) ? '#111' : '#ebf4fb'; const borderColour = dark ? 'darkslategray' : 'skyblue'; @@ -242,8 +242,8 @@ function attachKiwixPopoverCss(doc, dark) { -webkit-touch-callout: none !important; } `, - // The id of the style element for easy manipulation - 'kiwixtooltipstylesheet'); + // The id of the style element for easy manipulation + 'kiwixtooltipstylesheet'); } /** @@ -255,14 +255,14 @@ function attachKiwixPopoverCss(doc, dark) { * @param {ZIMArchive} archive The archive from which the popover information is extracted * @returns {Promise
} A Promise for the attached popover div or undefined if the popover is not attached */ -function populateKiwixPopoverDiv(ev, link, state, dark, archive) { +function populateKiwixPopoverDiv (ev, link, state, dark, archive) { // Do not show popover if the user has initiated an article load (set in filterClickEvent) if (link.articleisloading || link.popoverisloading) return Promise.resolve(); const linkHref = link.getAttribute('href'); // Do not show popover if there is no href or with certain landing pages if (!linkHref || /^wikivoyage/i.test(archive.file.name) && (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || - state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { + state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { return Promise.resolve(); } link.popoverisloading = true; @@ -331,7 +331,7 @@ function populateKiwixPopoverDiv(ev, link, state, dark, archive) { * @param {Event} event The event which has fired this popover action * @returns {Object} An object containing the popover div and the arrow span elements */ -function createNewKiwixPopoverCointainer(win, anchor, event) { +function createNewKiwixPopoverCointainer (win, anchor, event) { const div = document.createElement('div'); const linkHref = anchor.getAttribute('href'); const currentDocument = win.document; @@ -423,7 +423,7 @@ function createNewKiwixPopoverCointainer(win, anchor, event) { * @param {Element} popover The containing element of the popover (div) * @param {Document} doc The doucment on which to operate */ -function addEventListenersToPopoverIcons(anchor, popover, doc) { +function addEventListenersToPopoverIcons (anchor, popover, doc) { const breakout = function (e) { // Adding the newcontainer property to the anchor will be cauught by the filterClickEvent function and will open in new tab anchor.newcontainer = true; @@ -443,7 +443,7 @@ function addEventListenersToPopoverIcons(anchor, popover, doc) { * Remove any preview popover DIVs found in the given document * @param {Document} doc The document from which to remove any popovers */ -function removeKiwixPopoverDivs(doc) { +function removeKiwixPopoverDivs (doc) { const divs = doc.getElementsByClassName('kiwixtooltip'); // Timeout is set to allow for a delay before removing popovers - so user can hover the popover itself to prevent it from closing, // or so that links and buttons in the popover can be clicked @@ -471,7 +471,7 @@ function removeKiwixPopoverDivs(doc) { * Closes the specified popover div, with fadeout effect, and removes it from the DOM * @param {Element} div The div to close */ -function closePopover(div) { +function closePopover (div) { div.style.opacity = '0'; // Timeout allows the animation to complete before removing the div setTimeout(function () { From ec4a9d379828af133c5965f8fb252a945c7741a0 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Tue, 1 Oct 2024 17:33:04 +0530 Subject: [PATCH 03/10] Fixed the Popover issue again --- www/js/lib/popovers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index b1ec740a4..7d03b1519 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -261,8 +261,8 @@ function populateKiwixPopoverDiv (ev, link, state, dark, archive) { const linkHref = link.getAttribute('href'); // Do not show popover if there is no href or with certain landing pages if (!linkHref || /^wikivoyage/i.test(archive.file.name) && - (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || - state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { + (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || + state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { return Promise.resolve(); } link.popoverisloading = true; From ff594012829f9ee0bb5d845d160110ec436acca4 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Tue, 1 Oct 2024 17:33:49 +0530 Subject: [PATCH 04/10] Removing the extra space --- www/js/lib/popovers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 7d03b1519..c4a531152 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -261,7 +261,7 @@ function populateKiwixPopoverDiv (ev, link, state, dark, archive) { const linkHref = link.getAttribute('href'); // Do not show popover if there is no href or with certain landing pages if (!linkHref || /^wikivoyage/i.test(archive.file.name) && - (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || + (state.expectedArticleURLToBeDisplayed === archive.landingPageUrl || state.expectedArticleURLToBeDisplayed === 'A/Wikivoyage:Offline_reader_Expedition/Home_page')) { return Promise.resolve(); } From de761c5c20f0d619162a00fd3200f6cbe22733d4 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Sat, 12 Oct 2024 12:41:23 +0530 Subject: [PATCH 05/10] Fixed the changes requested --- www/js/lib/popovers.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index c4a531152..11eb5b1d3 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -104,19 +104,12 @@ function cleanUpLedeContent (node) { styleElements.forEach(style => { style.parentNode.removeChild(style); }); - const paragraphs = Array.from(node.querySelectorAll('p')); - // Filter out empty paragraphs or those with less than 50 characters + const paragraphs = Array.from(node.querySelectorAll('p:not(#pcs-edit-section-title-description)')); const parasWithContent = paragraphs.filter(para => { // DEV: Note that innerText is not supported in Firefox OS, so we need to use textContent as a fallback // The reason we prefer innerText is that it strips out hidden text and unnecessary whitespace, which is not the case with textContent const innerText = para.innerText ? para.innerText : para.textContent; - const text = innerText.trim(); - - // removing the para with less than 50 characters - // regex to check the paragraph if its too short or a brief description - const briefDescriptionRegex = /^.{1,100}$/; - - return !briefDescriptionRegex.test(text) && text.length >= 50; + return innerText.length >= 50; }); return parasWithContent; } From cbe064bde4643e599248c4683274b502cb27e336 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Sat, 12 Oct 2024 16:38:19 +0530 Subject: [PATCH 06/10] Restoring the line which shouldn't supposed to be removed --- www/js/lib/popovers.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 11eb5b1d3..1c4e9e3fc 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -109,7 +109,8 @@ function cleanUpLedeContent (node) { // DEV: Note that innerText is not supported in Firefox OS, so we need to use textContent as a fallback // The reason we prefer innerText is that it strips out hidden text and unnecessary whitespace, which is not the case with textContent const innerText = para.innerText ? para.innerText : para.textContent; - return innerText.length >= 50; + const text = innerText.trim(); + return !/^\s*$/.test(text) && text.length >= 50; }); return parasWithContent; } From 0a93973e3159c3fd8a96adb91da8dafedfbbfbd2 Mon Sep 17 00:00:00 2001 From: THEBOSS0369 Date: Sat, 12 Oct 2024 16:56:46 +0530 Subject: [PATCH 07/10] generalized the exclusion function logic for future --- www/js/lib/popovers.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 1c4e9e3fc..468b51470 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -99,12 +99,21 @@ function getArticleLede (href, baseUrl, articleDocument, archive) { // Helper function to clean up the lede content function cleanUpLedeContent (node) { + // Define an array of exclusion filters + // (note .exclude-this-class is a dummy class used as an exmple which will used to exclude classes in future) + const exclusionFilters = ['#pcs-edit-section-title-description', '.exclude-this-class']; + // Constrct the :not() selector thing + const notSelector = exclusionFilters.map(filter => `:not(${filter})`).join(''); + // Remove all standalone style elements from the given DOM node, because their content is shown by innerText and textContent const styleElements = Array.from(node.querySelectorAll('style')); styleElements.forEach(style => { style.parentNode.removeChild(style); }); - const paragraphs = Array.from(node.querySelectorAll('p:not(#pcs-edit-section-title-description)')); + // Apply this style-based exclusion filter to remove unwanted paragraphs in the popover + const paragraphs = Array.from(node.querySelectorAll(`p${notSelector}`)); + + // Filter out empty paragraphs or those with less than 50 character const parasWithContent = paragraphs.filter(para => { // DEV: Note that innerText is not supported in Firefox OS, so we need to use textContent as a fallback // The reason we prefer innerText is that it strips out hidden text and unnecessary whitespace, which is not the case with textContent From 80691a6f0e299376aed981b34159432cc7d0769a Mon Sep 17 00:00:00 2001 From: Anuj Kumar Sharma Date: Sat, 12 Oct 2024 20:51:15 +0530 Subject: [PATCH 08/10] Update www/js/lib/popovers.js Co-authored-by: Jaifroid --- www/js/lib/popovers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 468b51470..d6a679f70 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -100,7 +100,7 @@ function getArticleLede (href, baseUrl, articleDocument, archive) { // Helper function to clean up the lede content function cleanUpLedeContent (node) { // Define an array of exclusion filters - // (note .exclude-this-class is a dummy class used as an exmple which will used to exclude classes in future) + // (note `.exclude-this-class` is a dummy class used as an example for any future exclusion filters) const exclusionFilters = ['#pcs-edit-section-title-description', '.exclude-this-class']; // Constrct the :not() selector thing const notSelector = exclusionFilters.map(filter => `:not(${filter})`).join(''); From faefb34e0d1d3e9aae8bb0d1cafc6e5899c0eaf0 Mon Sep 17 00:00:00 2001 From: Anuj Kumar Sharma Date: Sat, 12 Oct 2024 20:51:26 +0530 Subject: [PATCH 09/10] Update www/js/lib/popovers.js Co-authored-by: Jaifroid --- www/js/lib/popovers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index d6a679f70..24dfeed66 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -102,7 +102,7 @@ function cleanUpLedeContent (node) { // Define an array of exclusion filters // (note `.exclude-this-class` is a dummy class used as an example for any future exclusion filters) const exclusionFilters = ['#pcs-edit-section-title-description', '.exclude-this-class']; - // Constrct the :not() selector thing + // Construct the `:not()` CSS exclusion selector list const notSelector = exclusionFilters.map(filter => `:not(${filter})`).join(''); // Remove all standalone style elements from the given DOM node, because their content is shown by innerText and textContent From 9ecd06df7ff40cfc961b97b6963a82d4bffc3b95 Mon Sep 17 00:00:00 2001 From: Anuj Kumar Sharma Date: Sat, 12 Oct 2024 20:51:39 +0530 Subject: [PATCH 10/10] Update www/js/lib/popovers.js Co-authored-by: Jaifroid --- www/js/lib/popovers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/js/lib/popovers.js b/www/js/lib/popovers.js index 24dfeed66..7155fe973 100644 --- a/www/js/lib/popovers.js +++ b/www/js/lib/popovers.js @@ -113,7 +113,7 @@ function cleanUpLedeContent (node) { // Apply this style-based exclusion filter to remove unwanted paragraphs in the popover const paragraphs = Array.from(node.querySelectorAll(`p${notSelector}`)); - // Filter out empty paragraphs or those with less than 50 character + // Filter out empty paragraphs or those with less than 50 characters const parasWithContent = paragraphs.filter(para => { // DEV: Note that innerText is not supported in Firefox OS, so we need to use textContent as a fallback // The reason we prefer innerText is that it strips out hidden text and unnecessary whitespace, which is not the case with textContent