Skip to content

allow multiple page rules #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
floooat wants to merge 1 commit into
base: master
Choose a base branch
from
Open

allow multiple page rules #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "gleap",
"version": "15.2.6",
"version": "15.2.7",
"main": "build/cjs/index.js",
"module": "build/esm/index.mjs",
"exports": {
Expand Down
6 changes: 3 additions & 3 deletions src/Gleap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import GleapAudioManager from './GleapAudioManager';
import GleapTagManager from './GleapTagManager';
import GleapAdminManager from './GleapAdminManager';
import GleapProductTours from './GleapProductTours';
import { checkPageFilter } from './GleapPageFilter';
import { checkPageFilter, checkPageRules } from './GleapPageFilter';
import { registerGleapChecklist } from './GleapChecklist';
import ChecklistNetworkManager from './ChecklistNetworkManager';

Expand Down Expand Up @@ -1241,8 +1241,8 @@ class Gleap {
for (let i = 0; i < actions.length; i++) {
const action = actions[i];
if (action && action.actionType) {
if (action.pageFilter && window && window.location) {
const passed = checkPageFilter(window.location.href, action.pageFilter, action.pageFilterType);
if ((action.pageRules?.length > 0 || action.pageFilter) && window && window.location) {
const passed = checkPageRules(window.location.href, action);

if (!passed) {
continue;
Expand Down
8 changes: 8 additions & 0 deletions src/GleapPageFilter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,11 @@ export const checkPageFilter = (currentUrl, pageFilter, pageFilterType) => {

return matched;
};

export const checkPageRules = (currentUrl, action) => {
const rules = action.pageRules && action.pageRules.length > 0
? action.pageRules
: (action.pageFilter ? [{ pageFilter: action.pageFilter, pageFilterType: action.pageFilterType }] : []);
if (rules.length === 0) return true;
return rules.every(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
Comment on lines +89 to +90
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rules.every(...) requires all rules to match the same currentUrl, which will make many realistic multi-rule configurations impossible (e.g., two 'is' rules for different pages can never both be true). If the intent of “multiple page rules” is “match any of these rules”, switch to rules.some(...), or add an explicit operator (e.g., match: 'any' | 'all') to avoid locking in the wrong semantics.

Suggested change
if (rules.length === 0) return true;
return rules.every(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
// Determine how multiple rules should be combined: 'any' (OR) or 'all' (AND).
const rulesMatchMode =
action && (action.pageRulesMatch === 'all' || action.pageRulesMatch === 'any')
? action.pageRulesMatch
: 'any';
if (rules.length === 0) return true;
if (rulesMatchMode === 'all') {
return rules.every(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
}
return rules.some(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with common JS style/lint rules and to avoid per-repo lint failures, consider adding parentheses around the arrow parameter ((r) => ...). This is especially relevant in shared libraries where consumers may run stricter linters.

Suggested change
return rules.every(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
return rules.every((r) => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));

Copilot uses AI. Check for mistakes.
};
6 changes: 5 additions & 1 deletion src/GleapTooltipManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { arrow, autoUpdate, computePosition, flip, offset, shift } from '@floating-ui/dom';
import { GleapSession } from './Gleap';
import { loadIcon } from './UI';
import { checkPageFilter } from './GleapPageFilter';
import { checkPageFilter, checkPageRules } from './GleapPageFilter';

export default class GleapTooltipManager {
tooltips = [];
Expand Down Expand Up @@ -427,6 +427,10 @@ export default class GleapTooltipManager {
const currentUrl = window.location.href;

return this.tooltips.filter((tooltip) => {
if (tooltip.pageRules && tooltip.pageRules.length > 0) {
return checkPageRules(currentUrl, { pageRules: tooltip.pageRules });
}
Comment on lines +430 to +432
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tooltip.pageRules is present, this early return bypasses the existing tooltip.page logic entirely. If pageRules is meant to extend (not replace) the legacy page behavior, consider combining them (e.g., evaluate both and apply a clearly defined AND/OR), or explicitly document/encode precedence (e.g., deprecate page when pageRules is set) to prevent unexpected behavior when both are configured.

Copilot uses AI. Check for mistakes.

if (!tooltip.page || tooltip.page.length === 0) {
return true;
}
Expand Down
Loading