Skip to content

Improve facet limitation #41

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
ThibaudGLT wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve facet limitation #41

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
58 changes: 48 additions & 10 deletions asset/js/facets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,43 @@ document.addEventListener('DOMContentLoaded', function () {
});
}

for (const button of document.querySelectorAll('.search-facet .search-facet-expand-button')) {
button.addEventListener('click', function() {
this.closest('.search-facet').classList.toggle('search-facet-expanded');
var buttonResetFacets = document.getElementById('reset-facets');
if (buttonResetFacets) {
buttonResetFacets.addEventListener('click', function () {
resetFacets();
});
}

var buttonToggleHiddenFacets = document.querySelectorAll('.show-hidden-facets-btn');
buttonToggleHiddenFacets.forEach(function (button) {
button.addEventListener('click', function () {
var facetName = button.getAttribute('data-facet-name');
toggleHiddenFacets(facetName, button);
});
});

function toggleHiddenFacets(name, button) {
var hiddenFacets = document.querySelector('.search-facet[data-facet-name="' + name + '"] .hidden-facets');

if (hiddenFacets.style.display === 'none' || hiddenFacets.style.display === '') {
hiddenFacets.style.display = 'block';
button.classList.remove('o-icon-down');
button.classList.add('o-icon-up');
button.innerHTML = ' ' + button.getAttribute('data-collapse-label');
} else {
hiddenFacets.style.display = 'none';
button.classList.remove('o-icon-up');
button.classList.add('o-icon-down');
button.innerHTML = ' ' + button.getAttribute('data-expand-label');
}
}
Comment on lines +24 to +38
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

C'est pas vraiment "theme-friendly": modif de l'icône et de la propriété display + le fait de se baser sur la propriété display pour savoir s'il faut cacher ou montrer le bloc.

Ça annule ce que j'avais fait dans 9c31e68 (où le JS se contente d'ajouter/retirer une classe, tout le reste est fait en CSS)

Pour le bouton qui change d'icône et de libellé, c'est à mon avis plus simple (et plus theme-able) d'avoir 2 boutons, et de cacher l'un ou l'autre selon le besoin.
Ça évite aussi une faille XSS (Si data-*-label contient une balise <script> par ex, c'est exécuté quand on clique sur le bouton)


function submitFacets() {
var params = new URLSearchParams(window.location.search);

const facetCheckboxes = Array.from(document.querySelectorAll('.search-facet-item input[type="checkbox"]'));
const facetNames = new Set(facetCheckboxes.map(node => node.dataset.facetName));
for (const [key, value] of params.entries()) {
for (const facetName of facetNames) {
if (key.startsWith(`limit[${facetName}]`)) {
params.delete(key);
}
for (const [key, value] of Array.from(params.entries())) {
if (key.startsWith('limit[')) {
params.delete(key);
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

En théorie on peut utiliser les paramètres limit pour autre chose que les facettes visibles.
Je pense que c'était mieux avant puisque ça ne supprimait que les paramètres des facettes visibles.
Pourquoi avoir changé d'ailleurs ? Ça ne marchait pas ?

}
}

Expand All @@ -34,4 +55,21 @@ document.addEventListener('DOMContentLoaded', function () {
});
window.location.search = params.toString();
}

function resetFacets() {
var allInputs = document.querySelectorAll('.search-facets input[type="checkbox"]:checked');
allInputs.forEach(input => {
input.checked = false;
});
Comment on lines +61 to +63
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

À quoi ça sert de décocher puisqu'on redirige juste après (location.search = ...) ?


var params = new URLSearchParams(window.location.search);
var newParams = new URLSearchParams();

for (let [key, value] of params.entries()) {
if (!key.startsWith('limit[')) {
newParams.append(key, value);
}
}
Comment on lines +68 to +72
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

Même remarque que pour submitFacets, on devrait conserver les paramètres limit qui ne correspondent pas à une facette visible

window.location.search = newParams.toString();
}
});
12 changes: 12 additions & 0 deletions src/Form/Admin/SearchPageEditForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ public function init()
],
]);

$settingsFieldset->add([
'name' => 'facet_limit_collapse_label',
'type' => 'Text',
'options' => [
'label' => 'Facet values collapse button label', // @translate
],
'attributes' => [
'value' => 'Show less', // @translate
'required' => true,
],
]);

$settingsFieldset->add([
'name' => 'facet_limit_expand_label',
'type' => 'Text',
Expand Down
10 changes: 7 additions & 3 deletions view/search/facets.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ $facetedFiltering = isset($settings['faceted_filtering']) && $settings['faceted_
<h2>
<?php echo $this->translate('Filters'); ?>
<?php if ($facetedFiltering) : ?>
<button class="fas fa-sync" id="submit-facets" title="<?php echo $this->translate('Filter with selected facets');?>"></button>
<button class="fas fa-filter" id="submit-facets" title="<?php echo $this->translate('Filter with selected facets');?>"></button>
<button class="fas fa-times" id="reset-facets" title="<?php echo $this->translate('Reset all filters');?>"></button>
Comment on lines +43 to +44
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

Il manque escapeHtml

<?php endif; ?>
</h2>
<ul>
Expand Down Expand Up @@ -69,8 +70,11 @@ $facetedFiltering = isset($settings['faceted_filtering']) && $settings['faceted_
</div>
<?php endif; ?>
<?php if (!empty($facetValues['hidden'])): ?>
<button class="search-facet-expand-button">
<?php echo $this->escapeHtml($this->searchCurrentPage()->settings()['facet_limit_expand_label']); ?>
<button id="show-hidden-facets-<?php echo $this->escapeHtml($name); ?>" class="o-icon-down show-hidden-facets-btn"
data-facet-name="<?php echo $this->escapeHtml($name); ?>"
data-expand-label="<?php echo $this->escapeHtml($this->searchCurrentPage()->settings()['facet_limit_expand_label']); ?>"
data-collapse-label="<?php echo $this->escapeHtml($this->searchCurrentPage()->settings()['facet_limit_collapse_label']); ?>">
&nbsp;<?php echo $this->escapeHtml($this->searchCurrentPage()->settings()['facet_limit_expand_label']); ?>
</button>
Comment on lines +75 to 78
Copy link
Copy Markdown
Member

@jajm jajm Mar 19, 2026

Choose a reason for hiding this comment

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

Tu peux utiliser $settings (déclaré plus haut) à la place de $this->searchCurrentPage()->settings()

<?php endif; ?>
</li>
Expand Down