-
Notifications
You must be signed in to change notification settings - Fork 33
New gallery type: grid #629
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
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| <?php | ||
|
|
||
| namespace App\Sites\Sections\Entries\Galleries; | ||
|
|
||
| class GalleryGridRenderService extends EntryGalleryRenderService | ||
| { | ||
| public function getViewData( | ||
| $entry, | ||
| $siteSettings, | ||
| $siteTemplateSettings, | ||
| $storageService, | ||
| $isEditMode, | ||
| $isLoopAvailable, | ||
| $asRowGallery, | ||
| $galleryItemsData, | ||
| $galleryItems, | ||
| $galleryType | ||
| ) { | ||
| $galleryItemsData = $this->getGalleryItemsData($entry); | ||
| $galleryItems = $this->generateGalleryItems($galleryItemsData, $entry, $storageService, $siteSettings); | ||
| $galleryType = isset($entry['mediaCacheData']['@attributes']['type']) ? $entry['mediaCacheData']['@attributes']['type'] : $siteTemplateSettings['entryLayout']['defaultGalleryType']; | ||
|
|
||
| $data = parent::getViewData( | ||
| $entry, | ||
| $siteSettings, | ||
| $siteTemplateSettings, | ||
| $storageService, | ||
| $isEditMode, | ||
| $isLoopAvailable, | ||
| $asRowGallery, | ||
| $galleryItemsData, | ||
| $galleryItems, | ||
| $galleryType | ||
| ); | ||
|
|
||
| $data['galleryClassList'] = $this->getGalleryClassList($galleryItemsData, $galleryType, $entry, $siteSettings); | ||
| $data['items'] = $galleryItems; | ||
| $data['gridColumnsMobile'] = $this->getGridColumns($entry, 'grid_columns_mobile', '1'); | ||
| $data['gridColumnsDesktop'] = $this->getGridColumns($entry, 'grid_columns_desktop', '2'); | ||
| $data['gridColumnsLargeDesktop'] = $this->getGridColumns($entry, 'grid_columns_large_desktop', '3'); | ||
| $data['gridGap'] = ! empty($entry['mediaCacheData']['@attributes']['grid_gap']) ? $entry['mediaCacheData']['@attributes']['grid_gap'] : false; | ||
|
|
||
| return $data; | ||
| } | ||
|
|
||
| public function getGalleryClassList($galleryItemsData, $galleryType, $entry, $siteSettings) | ||
| { | ||
| $classes = parent::getGalleryClassList($galleryItemsData, $galleryType, $entry, $siteSettings); | ||
|
|
||
| $classes[] = 'xGridShowCaptions-' . $this->getGridShowCaptions($entry); | ||
|
|
||
| return implode(' ', $classes); | ||
| } | ||
|
|
||
| private function getGridColumns($entry, $attribute, $default) | ||
| { | ||
| return ! empty($entry['mediaCacheData']['@attributes'][$attribute]) ? $entry['mediaCacheData']['@attributes'][$attribute] : $default; | ||
| } | ||
|
|
||
| private function getGridShowCaptions($entry) | ||
| { | ||
| return ! empty($entry['mediaCacheData']['@attributes']['grid_show_captions']) ? $entry['mediaCacheData']['@attributes']['grid_show_captions'] : 'yes'; | ||
| } | ||
|
|
||
| public function render( | ||
| $entry, | ||
| $siteSettings, | ||
| $siteTemplateSettings, | ||
| $storageService, | ||
| $isEditMode, | ||
| $isLoopAvailable, | ||
| $asRowGallery | ||
| ) { | ||
| if ($isEditMode && empty($entry['mediaCacheData']['file'])) { | ||
| return view('Sites/Sections/Entries/Galleries/editEmptyGallery'); | ||
| } | ||
|
|
||
| $data = $this->getViewData( | ||
| $entry, | ||
| $siteSettings, | ||
| $siteTemplateSettings, | ||
| $storageService, | ||
| $isEditMode, | ||
| $isLoopAvailable, | ||
| $asRowGallery, | ||
| null, | ||
| null, | ||
| null | ||
| ); | ||
|
|
||
| return view('Sites/Sections/Entries/Galleries/galleryGrid', $data); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <div class="{{ galleryClassList }}"{% if isFullscreen %} data-fullscreen="1"{% endif %}> | ||
| {% if items %} | ||
| <div class="xGallery" xGridColumnsMobile="{{ gridColumnsMobile }}" xGridColumnsDesktop="{{ gridColumnsDesktop }}" xGridColumnsLarge="{{ gridColumnsLargeDesktop }}"{% if gridGap %} xGridGap="{{ gridGap }}"{% endif %}> | ||
|
|
||
| {%- for item in items -%} | ||
| {%- if item.type == 'image' -%} | ||
| <div class="xGalleryItem xGalleryItemType-image xImgIndex-1"> | ||
| <img src="{{ item.src }}" width="{{ item.width }}" height="{{ item.height }}"{% if item.srcset %} srcset="{{ item.srcset }}"{% endif %} alt="{{ item.alt|raw }}"> | ||
|
Contributor
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. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline _api_app/app/Sites/Sections/Entries/Galleries/GalleryGridRenderService.php --items all
ast-grep outline editor/src/app/sites/sections/entries/galleries/gallery-grid-render.service.ts --items all
rg -n -C 5 'item\.alt|alt.*=>|alt:|htmlspecialchars|strip_tags|caption' \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryGridRenderService.php \
editor/src/app/sites/sections/entries/galleries/gallery-grid-render.service.tsRepository: berta-cms/berta Length of output: 3228 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository instructions ---'
find _api_app -name CLAUDE.md -print
for f in _api_app/CLAUDE.md _api_app/*/CLAUDE.md; do
[ -f "$f" ] && { printf '\n--- %s ---\n' "$f"; sed -n '1,220p' "$f"; }
done
printf '%s\n' '--- server renderer ---'
cat -n _api_app/app/Sites/Sections/Entries/Galleries/GalleryGridRenderService.php
printf '%s\n' '--- editor renderer ---'
cat -n editor/src/app/sites/sections/entries/galleries/gallery-grid-render.service.ts
printf '%s\n' '--- shared gallery data construction ---'
rg -n -C 8 'galleryItemsData|galleryItems|getViewData|alt' \
_api_app/app/Sites/Sections/Entries/Galleries \
editor/src/app/sites/sections/entries/galleries \
editor/src/app/render/twig-templates.tsRepository: berta-cms/berta Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- gallery service files ---'
git ls-files '_api_app/app/Sites/Sections/Entries/Galleries/*' \
'editor/src/app/sites/sections/entries/galleries/*'
printf '%s\n' '--- server base service ---'
rg -n -C 12 'class EntryGalleryRenderService|function generateGalleryItems|function getGalleryItemsData|alt' \
_api_app/app/Sites/Sections/Entries/Galleries
printf '%s\n' '--- editor base service ---'
rg -n -C 12 'class GalleryRenderService|generateGalleryItems|getGalleryItemsData|alt' \
editor/src/app/sites/sections/entries/galleriesRepository: berta-cms/berta Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- server gallery data path ---'
sed -n '33,58p' _api_app/app/Sites/Sections/Entries/Galleries/EntryGalleryRenderService.php
printf '%s\n' '--- server image helper definition ---'
rg -l 'function getGalleryItem|getGalleryItem\(' _api_app/app
for f in $(rg -l 'function getGalleryItem|getGalleryItem\(' _api_app/app); do
printf '\n--- %s ---\n' "$f"
rg -n -C 18 'function getGalleryItem|getGalleryItem\(' "$f"
done
printf '%s\n' '--- editor item construction, exact range ---'
sed -n '11,90p' editor/src/app/sites/sections/entries/galleries/gallery-render.service.tsRepository: berta-cms/berta Length of output: 16774 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete server alt construction ---'
sed -n '17,95p' _api_app/app/Shared/ImageHelpers.php
printf '%s\n' '--- gallery metadata input references ---'
rg -n -C 6 "mediaCacheData|`@value`|caption|gallery" \
_api_app/app/Sites/Sections/Entries/SectionEntriesDataService.php \
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts \
editor/src/app/sites/sections/entries/galleries/gallery-grid-render.service.tsRepository: berta-cms/berta Length of output: 50373 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- gallery caption writes ---'
rg -n -C 8 "mediaCacheData.*file|file.*`@value`|\\['`@value`'\\].*=|gallery.*caption|caption.*gallery" \
_api_app/app editor/src/app \
-g '*.php' -g '*.ts' -g '*.html' -g '*.twig' \
| head -n 240
printf '%s\n' '--- entry update methods ---'
rg -n "^ (public|private|protected) function |`@value`|mediaCacheData" \
_api_app/app/Sites/Sections/Entries/SectionEntriesDataService.php \
| head -n 260Repository: berta-cms/berta Length of output: 25043 XSS (CWE-79): Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') Exploitability: Moderate Escape Both gallery services derive 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| <div class="xGalleryImageCaption">{{ item.caption|raw }}</div> | ||
| </div> | ||
| {%- else -%} | ||
| <div class="xGalleryItem xGalleryItemType-video"> | ||
| <video width="{{ item.width }}" controls controlsList="nodownload"{% if item.poster %} poster="{{ item.poster }}"{% endif %}{% if item.autoplay %} data-autoplay="1"{% endif %}> | ||
| <source src="{{ item.original }}" type="video/mp4"> | ||
| </video> | ||
| <div class="xGalleryImageCaption">{{ item.caption|raw }}</div> | ||
| </div> | ||
| {%- endif -%} | ||
| {%- endfor -%} | ||
|
|
||
| {%- if isEditMode -%} | ||
| <a href="#" class="xGalleryEditButton xEditorLink xSysCaption xMAlign-container"> | ||
| <span class="xMAlign-outer-gallery"> | ||
| <span class="xMAlign-inner-gallery">edit gallery</span> | ||
| </span> | ||
| </a> | ||
| {%- endif -%} | ||
| </div> | ||
|
|
||
| <ul class="xGalleryNav" style="display:none"> | ||
| {% for item in navigationItems %} | ||
| <li> | ||
| <a href="{{ item.src }}" target="_blank"{{ item.attributes|raw }}> | ||
| <span>{{ item.index }}</span> | ||
| </a> | ||
| <div class="xGalleryImageCaption">{{ item.caption|raw }}</div> | ||
| </li> | ||
| {% endfor %} | ||
| </ul> | ||
| {% endif %} | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| use App\Plugins\Shop\ShopSettingsDataService; | ||
| use App\Shared\Helpers; | ||
| use App\Sites\Sections\Entries\Galleries\GalleryColumnRenderService; | ||
| use App\Sites\Sections\Entries\Galleries\GalleryGridRenderService; | ||
| use App\Sites\Sections\Entries\Galleries\GalleryLinkRenderService; | ||
| use App\Sites\Sections\Entries\Galleries\GalleryPileRenderService; | ||
| use App\Sites\Sections\Entries\Galleries\GalleryRowRenderService; | ||
|
|
@@ -22,13 +23,16 @@ class SectionEntryRenderService | |
|
|
||
| private $galleryLinkRenderService; | ||
|
|
||
| private $galleryGridRenderService; | ||
|
|
||
| public function __construct() | ||
| { | ||
| $this->gallerySlideshowRenderService = new GallerySlideshowRenderService; | ||
| $this->galleryRowRenderService = new GalleryRowRenderService($this->gallerySlideshowRenderService); | ||
| $this->galleryColumnRenderService = new GalleryColumnRenderService; | ||
| $this->galleryPileRenderService = new GalleryPileRenderService($this->gallerySlideshowRenderService); | ||
| $this->galleryLinkRenderService = new GalleryLinkRenderService; | ||
| $this->galleryGridRenderService = new GalleryGridRenderService; | ||
|
Comment on lines
+26
to
+35
Contributor
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. 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- applicable repository instructions ---'
if [ -f _api_app/CLAUDE.md ]; then
cat _api_app/CLAUDE.md
fi
printf '%s\n' '--- target service ---'
cat -n _api_app/app/Sites/Sections/Entries/SectionEntryRenderService.php
printf '%s\n' '--- related renderer constructors and bindings ---'
rg -n -A12 -B3 'class (GallerySlideshowRenderService|GalleryRowRenderService|GalleryColumnRenderService|GalleryPileRenderService|GalleryLinkRenderService|GalleryGridRenderService)|function __construct|SectionEntryRenderService' _api_app/app _api_app/routes _api_app/bootstrap 2>/dev/nullRepository: berta-cms/berta Length of output: 50371 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- exact change ---'
git diff --unified=20 -- _api_app/app/Sites/Sections/Entries/SectionEntryRenderService.php
printf '%s\n' '--- gallery renderer definitions ---'
for file in _api_app/app/Sites/Sections/Entries/Galleries/GallerySlideshowRenderService.php \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryRowRenderService.php \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryColumnRenderService.php \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryPileRenderService.php \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryLinkRenderService.php \
_api_app/app/Sites/Sections/Entries/Galleries/GalleryGridRenderService.php; do
if [ -f "$file" ]; then
printf '\n--- %s ---\n' "$file"
sed -n '1,80p' "$file"
fi
done
printf '%s\n' '--- direct construction of SectionEntryRenderService ---'
rg -n -C3 'new SectionEntryRenderService|SectionEntryRenderService::class|function __construct' _api_app/app/Sites _api_app/routes | head -160Repository: berta-cms/berta Length of output: 30641 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- repository state for the reviewed file ---'
git status --short -- _api_app/app/Sites/Sections/Entries/SectionEntryRenderService.php
git diff --name-status
git diff --cached --name-status
git diff HEAD -- _api_app/app/Sites/Sections/Entries/SectionEntryRenderService.php
printf '%s\n' '--- scoped repository knowledge files ---'
find /tmp/coderabbit-repo-knowledge/berta-cms-berta-d1db947b -maxdepth 2 -type f -path '*/conventions/*' -o -path '*/learnings/*' -o -path '*/architecture/*' | sort | head -80Repository: berta-cms/berta Length of output: 1041 Inject the gallery renderers through typed constructor properties.
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -80,6 +84,10 @@ public function getViewData( | |
| $galleryTypeRenderService = $this->galleryLinkRenderService; | ||
| break; | ||
|
|
||
| case 'grid': | ||
| $galleryTypeRenderService = $this->galleryGridRenderService; | ||
| break; | ||
|
|
||
| default: | ||
| // slideshow | ||
| $galleryTypeRenderService = $this->gallerySlideshowRenderService; | ||
|
|
||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,6 @@ h1 { | |
|
|
||
| .xGallery { | ||
| position: relative; | ||
| display: block; | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: berta-cms/berta
Length of output: 24822
🏁 Script executed:
Repository: berta-cms/berta
Length of output: 25956
🌐 Web query:
PHP 8.4 method signature inheritance compatibility adding parameter type or return type when parent method is untyped contravariance covariance official documentation💡 Result:
In PHP 8.4, the rules for method signature inheritance are governed by the Liskov Substitution Principle (LSP), which ensures that a child class method remains compatible with the parent class method it overrides [1]. When a parent method is untyped (i.e., it lacks explicit parameter or return type declarations), the inheritance rules allow for the addition of types in the overriding child method, provided they adhere to variance principles [1][2]. 1. Return Type Covariance: You can add a return type to a child method that overrides an untyped parent method. Because the parent is untyped (effectively allowing any return), narrowing this to a specific type in the child is considered covariant, which is allowed [2][3]. 2. Parameter Type Contravariance: You can add a parameter type to a child method that overrides an untyped parent method. Because the parent parameter is untyped (effectively accepting any type), specifying a type in the child is a form of restriction. Technically, for full contravariance compliance, the child method should accept a type that is equal to or wider than the parent's requirement [2][3]. Since an untyped parameter acts as accepting mixed/any, introducing a specific type in the child parameter is generally compatible because the child remains substitutable for the parent in scenarios where the parent is used, although it effectively restricts the input type [2][3]. Important Exceptions and Nuances: - Internal Methods: A critical exception exists for internal (built-in) PHP methods. Many internal methods have "tentatively" declared types since PHP 8.1 [4][5]. If you override an internal method that now has a tentative signature, your child method must be compatible with that signature, or you must use the #[\ReturnTypeWillChange] attribute to suppress deprecation notices [4][6][5]. - Fatal Errors: If a child method signature violates these compatibility rules (e.g., uses an incompatible return type), PHP will emit a fatal error [1]. - Visibility: While adding types is allowed, you cannot restrict the visibility of the method in the child class (e.g., changing a public method to private is forbidden) [1][6]. In summary, for user-defined classes, adding type declarations to an otherwise untyped parent method signature is permitted under PHP's variance rules [1][2]. Always ensure that any added types follow standard covariance for returns and contravariance for parameters [2][3].
Citations:
🏁 Script executed:
Repository: berta-cms/berta
Length of output: 43032
Define the gallery renderer contract across the hierarchy.
GalleryGridRenderServiceneeds parameter types, return types, and array-shape PHPDoc. UpdateEntryGalleryRenderServiceand all gallery overrides together because concrete parameter types cannot be added only to this override. Remove the unusedgetViewData()placeholders or make them nullable;render()currently passesnullfor them.🤖 Prompt for AI Agents
Source: Coding guidelines