-
Notifications
You must be signed in to change notification settings - Fork 385
Accessibility: Removes hard-coded columns in home facet lists #4302
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
ce69280
d6fd384
3ff7813
ddb59ed
1740801
1ee6f93
79ceff0
0835aa5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -119,7 +119,41 @@ public function setConfig($settings) | |||||
| $this->searchClassId = empty($parts[0]) ? $this->searchClassId : $parts[0]; | ||||||
| $this->columnSize = $parts[1] ?? $this->columnSize; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| /** | ||||||
| * Get list of facet fields that should be displayed in two columns on the homepage | ||||||
| * (configured via facets.ini -> [HomePage_Settings] -> two_column_facets). | ||||||
| * | ||||||
| * @param Config $facetConfig Facet configuration object. | ||||||
| * | ||||||
| * @return string[] | ||||||
| */ | ||||||
| protected function getTwoColumnFacets(Config $facetConfig): array | ||||||
| { | ||||||
| $raw = null; | ||||||
|
|
||||||
| if (isset($facetConfig->HomePage_Settings) | ||||||
| && isset($facetConfig->HomePage_Settings['two_column_facets']) | ||||||
| ) { | ||||||
| $raw = $facetConfig->HomePage_Settings['two_column_facets']; | ||||||
| } | ||||||
|
|
||||||
| if (null === $raw) { | ||||||
| return []; | ||||||
| } | ||||||
|
|
||||||
| // Laminas config may return string or array depending on ini syntax; normalize: | ||||||
| if (is_array($raw)) { | ||||||
| $items = $raw; | ||||||
| } else { | ||||||
| $items = preg_split('/\s*,\s*/', (string)$raw) ?: []; | ||||||
| } | ||||||
|
|
||||||
| $items = array_map('trim', $items); | ||||||
| $items = array_filter($items, fn ($v) => $v !== ''); | ||||||
|
|
||||||
| return array_values(array_unique($items)); | ||||||
| } | ||||||
| /** | ||||||
| * Return context variables used for rendering the block's template. | ||||||
| * | ||||||
|
|
@@ -138,6 +172,7 @@ public function getContext() | |||||
| 'hierarchicalFacets' => $this->getHierarchicalFacets($facetConfig), | ||||||
| 'hierarchicalFacetSortOptions' => | ||||||
| $this->getHierarchicalFacetSortSettings($facetConfig), | ||||||
| 'twoColumnFacets' => $this->getTwoColumnFacets($facetConfig), | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that you can simplify this by adding: at the top of the class definition, and then changing this line to:
Suggested change
This existing trait does much the same thing as your getTwoColumnFacets method, so we might as well use the standard logic instead of creating a new variation. |
||||||
| 'results' => $results, | ||||||
| ]; | ||||||
| } | ||||||
|
|
||||||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,9 +9,12 @@ | |||||
| <?php if (!empty($facetList)): ?> | ||||||
| <div class="search-home-facets"> | ||||||
| <?php foreach ($facetList as $field => $details): ?> | ||||||
| <?php $isHierarchy = in_array($field, $hierarchicalFacets ?? []); ?> | ||||||
| <?php | ||||||
| $isHierarchy = in_array($field, $hierarchicalFacets ?? []); | ||||||
| $useTwoColumns = in_array($field, $twoColumnFacets ?? [], true); | ||||||
| ?> | ||||||
| <?php $labelHeading = $this->transEsc('home_browse_by_facet', ['%%facet%%' => $this->translate($details['label'])]); ?> | ||||||
| <div class="home-facet <?=$this->escapeHtmlAttr($field) ?><?=$isHierarchy ? ' facet-tree' : ''?>"> | ||||||
| <div class="home-facet <?=$this->escapeHtmlAttr($field) ?><?=$isHierarchy ? ' facet-tree' : ''?><?=$useTwoColumns ? ' two-columns' : ''?>"> | ||||||
| <h2><?=$labelHeading?></h2> | ||||||
| <div class="home-facet-container"> | ||||||
| <ul class="home-facet-list"> | ||||||
|
|
@@ -27,9 +30,9 @@ | |||||
| <?php | ||||||
| $sortedList = $this->sortFacetList($results, $field, $details['list'], $basicSearch, $format); | ||||||
|
|
||||||
| // Special case: two columns for LC call numbers... | ||||||
| $maxListLength = $field == 'callnumber-first' | ||||||
| ? $columnSize * 2 : $columnSize; | ||||||
| // Two-column facets (configured in facets.ini: [HomePage_Settings] two_column_facets): | ||||||
| //$useTwoColumns = in_array($field, $twoColumnFacets ?? [], true); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this supposed to be commented out? I suspect not:
Suggested change
|
||||||
| $maxListLength = $useTwoColumns ? $columnSize * 2 : $columnSize; | ||||||
|
|
||||||
| // Special case: custom URLs for collections... | ||||||
| $moreUrl = $field == 'hierarchy_top_title' | ||||||
|
|
@@ -56,8 +59,6 @@ | |||||
| <?php if ($i >= $maxListLength): // list too long? show more link! ?> | ||||||
| <li><a href="<?=$moreUrl?>"><strong><?=$this->transEsc('more_options_ellipsis')?></strong></a></li> | ||||||
| <?php break; ?> | ||||||
| <?php elseif ($i % $columnSize === 0): // end of column? insert break! ?> | ||||||
| </ul><ul class="home-facet-list"> | ||||||
| <?php endif; ?> | ||||||
| <?php endforeach; ?> | ||||||
| <?php endif; ?> | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was uncommented by accident: