Skip to content

Commit b4ba895

Browse files
author
Callin Mullaney
committed
fix(theme): clean favicon settings persistence
1 parent 3f61132 commit b4ba895

7 files changed

Lines changed: 78 additions & 32 deletions

File tree

config/schema/emulsify.schema.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ emulsify.settings:
1414
type: integer
1515
favicon_source_svg:
1616
type: text
17-
label: 'Exportable SVG source markup'
17+
label: 'Portable SVG source markup'
1818
favicon_source_filename:
1919
type: string
2020
label: 'Source SVG filename'

src/Favicon/FaviconPackageGenerator.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public function generate(string $theme_name, File $source_file, array $settings)
168168
}
169169

170170
/**
171-
* Generates a full favicon package from exportable SVG source markup.
171+
* Generates a full favicon package from portable SVG source markup.
172172
*
173173
* @param array<string, mixed> $source
174174
* Optional source metadata such as file ID or filename.
@@ -430,6 +430,8 @@ private function encodeJson(mixed $data, int $options = 0): string {
430430
*/
431431
private function buildPackageHash(array $settings, string $source_hash): string {
432432
$hash_settings = $settings;
433+
// File IDs, stored source copies, and human-facing labels do not change the
434+
// rendered package output, so exclude them from the deterministic hash.
433435
unset(
434436
$hash_settings['favicon_source_fid'],
435437
$hash_settings['favicon_source_svg'],
@@ -506,7 +508,7 @@ private function sanitizeSvg(string $source_data): string {
506508
* Inspects and sanitizes SVG markup.
507509
*
508510
* @return array<string, mixed>
509-
* Source analysis including the sanitized SVG.
511+
* Source analysis including sanitized markup and non-fatal warnings.
510512
*/
511513
private function inspectSvgMarkup(string $source_data): array {
512514
$document = $this->loadSvgDocument($source_data);
@@ -808,6 +810,8 @@ private function renderPng(string $mime_type, string $source_data, int $size, st
808810
$background = imagecolorallocatealpha($canvas, $red, $green, $blue, 0);
809811
imagefilledrectangle($canvas, 0, 0, $size, $size, $background);
810812

813+
// All raster outputs share the same centering and padding rules so the
814+
// browser, iOS, and Android assets stay visually aligned.
811815
$inner_size = max(1, $size - (2 * (int) round($size * ($padding / 100))));
812816
$scale = min($inner_size / $source_width, $inner_size / $source_height);
813817
$target_width = max(1, (int) round($source_width * $scale));

src/Favicon/FaviconPreviewBuilder.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ private function resolvePreviewSourceUrl(string $platform, array $settings, ?Fil
172172
return $this->fileUrlGenerator->generateString($source_file->getFileUri());
173173
}
174174

175-
if (FaviconSettings::hasExportableSource($settings)) {
176-
return 'data:image/svg+xml;base64,' . base64_encode(FaviconSettings::getSourceSvg($settings));
175+
if (FaviconSettings::hasPortableSource($settings)) {
176+
return 'data:image/svg+xml;base64,' . base64_encode(FaviconSettings::getPortableSourceSvg($settings));
177177
}
178178

179179
$package_path = $settings['favicon_package_path'] ?? '';

src/Favicon/FaviconSettings.php

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,35 @@ public static function getSourceFileId(array $settings): ?int {
121121
}
122122

123123
/**
124-
* Returns the exportable SVG source stored in config.
124+
* Returns the portable SVG source stored in config.
125125
*/
126-
public static function getSourceSvg(array $settings): string {
126+
public static function getPortableSourceSvg(array $settings): string {
127127
return trim((string) ($settings['favicon_source_svg'] ?? ''));
128128
}
129129

130130
/**
131-
* Indicates whether an exportable SVG source is available.
131+
* Indicates whether a portable SVG source is available.
132+
*/
133+
public static function hasPortableSource(array $settings): bool {
134+
return self::getPortableSourceSvg($settings) !== '';
135+
}
136+
137+
/**
138+
* Returns the portable SVG source stored in config.
139+
*
140+
* @todo Remove in Emulsify 8.x after callers move to getPortableSourceSvg().
141+
*/
142+
public static function getSourceSvg(array $settings): string {
143+
return self::getPortableSourceSvg($settings);
144+
}
145+
146+
/**
147+
* Indicates whether a portable SVG source is available.
148+
*
149+
* @todo Remove in Emulsify 8.x after callers move to hasPortableSource().
132150
*/
133151
public static function hasExportableSource(array $settings): bool {
134-
return self::getSourceSvg($settings) !== '';
152+
return self::hasPortableSource($settings);
135153
}
136154

137155
/**

src/Hook/FaviconHooks.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ private function resolveRuntimePackageSettings(string $theme_name, array $settin
106106
* Resolves package settings when a portable SVG source is available.
107107
*/
108108
private function doResolveRuntimePackageSettings(string $theme_name, array $settings): ?array {
109-
$source_svg = FaviconSettings::getSourceSvg($settings);
109+
$source_svg = FaviconSettings::getPortableSourceSvg($settings);
110110
$source_metadata = [
111111
'filename' => (string) ($settings['favicon_source_filename'] ?? 'favicon.svg'),
112112
];

src/Hook/ThemeSettingsHooks.php

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,26 @@ final class ThemeSettingsHooks {
2727

2828
use StringTranslationTrait;
2929

30+
/**
31+
* Theme settings UI keys that should never be persisted to config.
32+
*
33+
* ThemeSettingsForm saves arbitrary submitted values unless they are marked
34+
* as clean, so all helper-only UI keys live here.
35+
*/
36+
private const NON_CONFIG_THEME_SETTING_KEYS = [
37+
'portable_source',
38+
'portable_source_notice',
39+
'diagnostics',
40+
'source_diagnostics_notice',
41+
'status',
42+
'package_status_notice',
43+
'maskable_info',
44+
'maskable_icon_notice',
45+
'generation_hint',
46+
'dirty_state',
47+
'emulsify_tools_apply_admin_theme_favicon',
48+
];
49+
3050
/**
3151
* Generates favicon packages from uploaded theme assets.
3252
*/
@@ -115,6 +135,7 @@ public function formSystemThemeSettingsAlter(array &$form, FormStateInterface $f
115135
$form['#attached']['library'][] = 'emulsify/favicon_admin';
116136
$form['#validate'][] = [self::class, 'validateFaviconSettings'];
117137
$form['#submit'][] = [self::class, 'submitFaviconSettings'];
138+
$this->registerCleanValueKeys($form_state);
118139

119140
$form['favicon_package_hash'] = [
120141
'#type' => 'hidden',
@@ -189,7 +210,7 @@ public function formSystemThemeSettingsAlter(array &$form, FormStateInterface $f
189210
],
190211
'#description' => $this->t('Use a square SVG with a square viewBox. Embedded base64 raster image data is allowed, but it may scale less cleanly than a pure vector source.'),
191212
];
192-
$form['emulsify_favicon']['source']['portable_source'] = [
213+
$form['emulsify_favicon']['source']['portable_source_notice'] = [
193214
'#type' => 'item',
194215
'#title' => $this->t('Portable source config'),
195216
'#markup' => '<span class="description">' . $this->buildPortableSourceDescription($package_status) . '</span>',
@@ -211,14 +232,14 @@ public function formSystemThemeSettingsAlter(array &$form, FormStateInterface $f
211232
];
212233
}
213234
if ($package_status['source_diagnostics'] !== '') {
214-
$form['emulsify_favicon']['source']['diagnostics'] = [
235+
$form['emulsify_favicon']['source']['source_diagnostics_notice'] = [
215236
'#type' => 'item',
216237
'#title' => $this->t('Source diagnostics'),
217238
'#markup' => $package_status['source_diagnostics'],
218239
];
219240
}
220241

221-
$form['emulsify_favicon']['status'] = [
242+
$form['emulsify_favicon']['package_status_notice'] = [
222243
'#type' => 'item',
223244
'#title' => $this->t('Current generated package'),
224245
'#markup' => $package_status['status_markup'],
@@ -304,7 +325,7 @@ public function formSystemThemeSettingsAlter(array &$form, FormStateInterface $f
304325
'#max' => 40,
305326
'#description' => $this->t('The maskable icon keeps at least 20% padding to protect the safe area.'),
306327
];
307-
$form['emulsify_favicon']['android']['maskable_info'] = [
328+
$form['emulsify_favicon']['android']['maskable_icon_notice'] = [
308329
'#type' => 'item',
309330
'#title' => $this->t('Maskable Android Icon'),
310331
'#markup' => '<span class="description">' . $this->t('Emulsify always generates a dedicated maskable Android icon with extra safe-area padding so launchers can crop the outer edges into circles or squircles without cutting into the logo.') . '</span>',
@@ -335,8 +356,8 @@ public function formSystemThemeSettingsAlter(array &$form, FormStateInterface $f
335356

336357
$form['emulsify_favicon']['actions']['regenerate_package'] = [
337358
'#type' => 'submit',
359+
'#name' => 'regenerate_package',
338360
'#value' => $default_label,
339-
'#submit' => [[self::class, 'requestFaviconRegeneration']],
340361
'#button_type' => $package_status['state'] === 'current' ? 'secondary' : 'primary',
341362
'#attributes' => [
342363
'data-favicon-regenerate-button' => 'true',
@@ -381,13 +402,6 @@ public static function submitFaviconSettings(array &$form, FormStateInterface $f
381402
self::service()->doSubmitFaviconSettings($form_state);
382403
}
383404

384-
/**
385-
* Static generate button callback.
386-
*/
387-
public static function requestFaviconRegeneration(array &$form, FormStateInterface $form_state): void {
388-
self::service()->doRequestFaviconRegeneration($form_state);
389-
}
390-
391405
/**
392406
* Static reset button callback.
393407
*/
@@ -396,10 +410,12 @@ public static function resetFaviconSettings(array &$form, FormStateInterface $fo
396410
}
397411

398412
/**
399-
* Flags the form submit as an explicit regeneration request.
413+
* Marks helper-only values so ThemeSettingsForm does not export them.
400414
*/
401-
private function doRequestFaviconRegeneration(FormStateInterface $form_state): void {
402-
$form_state->set('emulsify_favicon_force_generate', TRUE);
415+
private function registerCleanValueKeys(FormStateInterface $form_state): void {
416+
foreach (self::NON_CONFIG_THEME_SETTING_KEYS as $key) {
417+
$form_state->addCleanValueKey($key);
418+
}
403419
}
404420

405421
/**
@@ -479,7 +495,7 @@ private function doSubmitFaviconSettings(FormStateInterface $form_state): void {
479495
$source_context['source_svg'],
480496
);
481497
$metadata = $this->packageGenerator->readPackageMetadata($definition['path']);
482-
$should_generate = (bool) $form_state->get('emulsify_favicon_force_generate')
498+
$should_generate = $this->isRegenerationRequest($form_state)
483499
|| $settings['favicon_package_hash'] !== $definition['hash']
484500
|| $settings['favicon_package_path'] !== $definition['path']
485501
|| !$this->packageGenerator->packageExists($definition['path']);
@@ -554,6 +570,14 @@ private function doResetFaviconSettings(FormStateInterface $form_state): void {
554570
$this->messenger->addStatus($this->t('Reset the generated favicon package and restored the theme default favicon behavior.'));
555571
}
556572

573+
/**
574+
* Returns whether the current submit was an explicit regeneration request.
575+
*/
576+
private function isRegenerationRequest(FormStateInterface $form_state): bool {
577+
$trigger = $form_state->getTriggeringElement();
578+
return is_array($trigger) && ($trigger['#name'] ?? '') === 'regenerate_package';
579+
}
580+
557581
/**
558582
* Resolves a stored managed file source when it still exists.
559583
*/
@@ -574,7 +598,7 @@ private function resolveStoredSourceFile(array $settings): ?File {
574598
* The resolved source context, or an empty array if no source exists.
575599
*/
576600
private function resolveSourceContext(array $settings, bool $requires_rasterization): array {
577-
$source_svg = FaviconSettings::getSourceSvg($settings);
601+
$source_svg = FaviconSettings::getPortableSourceSvg($settings);
578602
$source_filename = (string) ($settings['favicon_source_filename'] ?: 'favicon.svg');
579603
$source_fid = FaviconSettings::getSourceFileId($settings);
580604
$source_file = $source_fid ? File::load($source_fid) : NULL;
@@ -638,7 +662,7 @@ private function buildPackageStatus(string $theme_name, array $settings, ?File $
638662
'package_exists' => !empty($settings['favicon_package_path']) && $this->packageGenerator->packageExists($settings['favicon_package_path']),
639663
'source_available' => FALSE,
640664
'portable_source_missing' => FALSE,
641-
'exportable_source' => FaviconSettings::hasExportableSource($settings),
665+
'portable_source_available' => FaviconSettings::hasPortableSource($settings),
642666
'source_diagnostics' => '',
643667
'status_markup' => '',
644668
];
@@ -647,9 +671,9 @@ private function buildPackageStatus(string $theme_name, array $settings, ?File $
647671
$source_context = [];
648672
if ($source_file) {
649673
$source_context = $this->resolveSourceContext($settings, FALSE);
650-
$status['portable_source_missing'] = !$status['exportable_source'];
674+
$status['portable_source_missing'] = !$status['portable_source_available'];
651675
}
652-
elseif ($status['exportable_source']) {
676+
elseif ($status['portable_source_available']) {
653677
$source_context = $this->resolveSourceContext($settings, FALSE);
654678
}
655679

@@ -696,7 +720,7 @@ private function buildPackageStatus(string $theme_name, array $settings, ?File $
696720
* Describes whether the current source is portable through config.
697721
*/
698722
private function buildPortableSourceDescription(array $package_status): string {
699-
if ($package_status['exportable_source']) {
723+
if ($package_status['portable_source_available']) {
700724
return (string) $this->t('The sanitized SVG source is saved in theme config and can be regenerated after configuration import.');
701725
}
702726

whisk/config/schema/whisk.schema.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ whisk.settings:
1414
type: integer
1515
favicon_source_svg:
1616
type: text
17-
label: 'Exportable SVG source markup'
17+
label: 'Portable SVG source markup'
1818
favicon_source_filename:
1919
type: string
2020
label: 'Source SVG filename'

0 commit comments

Comments
 (0)