Skip to content

Commit 2e70042

Browse files
committed
Apply fixed Coderabbitai review.
1 parent 7990d5c commit 2e70042

7 files changed

Lines changed: 167 additions & 59 deletions

File tree

‎resources/assets/dist/js/toolbar.min.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎resources/src/toolbar/element.js‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,29 @@ export function YiiDebugToolbar() {
8888
self.boundPointerUp = self.onPointerUp.bind(self);
8989
self.boundHostThemeRefresh = self.refreshHostThemeControl.bind(self);
9090
self.boundThemeRefresh = self.refreshTheme.bind(self);
91+
/**
92+
* A pointer landing anywhere but the menu dismisses it. Built here with the
93+
* other bound handlers so the registration can follow the element lifecycle:
94+
* `connectedCallback()` attaches it to the document and
95+
* `disconnectedCallback()` detaches it again.
96+
*/
97+
self.boundExtensionsPointerDown = function (event) {
98+
if (!self.extensionsOpen) {
99+
return;
100+
}
101+
102+
/**
103+
* Listening on the document means `event.target` is retargeted to the
104+
* host element; the composed path still carries the real one.
105+
*/
106+
var path =
107+
typeof event.composedPath === "function" ? event.composedPath() : [];
108+
var target = path.length > 0 ? path[0] : event.target;
109+
110+
if (!isInsideExtensions(target, closest)) {
111+
self.closeExtensions(false);
112+
}
113+
};
91114
self.theme = null;
92115
self.themeObserver = null;
93116
self.themeRefreshTimer = null;
@@ -107,6 +130,12 @@ YiiDebugToolbar.prototype.connectedCallback = function () {
107130
toolbars.push(this);
108131
}
109132

133+
document.addEventListener(
134+
"pointerdown",
135+
this.boundExtensionsPointerDown,
136+
false,
137+
);
138+
110139
resetHostThemeControlCache();
111140
this.ownsTheme = !hostHasThemeControl();
112141
this.refreshTheme();
@@ -163,6 +192,11 @@ YiiDebugToolbar.prototype.disconnectedCallback = function () {
163192
this.resizing = false;
164193
document.removeEventListener("pointermove", this.boundPointerMove, false);
165194
document.removeEventListener("pointerup", this.boundPointerUp, false);
195+
document.removeEventListener(
196+
"pointerdown",
197+
this.boundExtensionsPointerDown,
198+
false,
199+
);
166200
};
167201

168202
YiiDebugToolbar.prototype.setAjaxRequests = function (requests) {
@@ -877,7 +911,7 @@ YiiDebugToolbar.prototype.renderExtensions = function (extensions) {
877911
(open ? " is-open" : "") +
878912
'"><button type="button" class="panel extensions-toggle' +
879913
(active ? " panel-active" : "") +
880-
'" aria-haspopup="true" aria-expanded="' +
914+
'" aria-expanded="' +
881915
(open ? "true" : "false") +
882916
'" aria-controls="extensions-menu" title="Extensions">' +
883917
this.iconHtml("dots", "panel-icon") +
@@ -1227,37 +1261,6 @@ YiiDebugToolbar.prototype.bindDelegatedEvents = function () {
12271261
event.stopPropagation();
12281262
self.closeDrawer();
12291263
});
1230-
1231-
/**
1232-
* A pointer landing anywhere but the menu dismisses it. Registered once per
1233-
* element — `ensureShadowSkeleton()` binds the delegates a single time, and
1234-
* the guard keeps a re-entry from stacking duplicates.
1235-
*/
1236-
if (!this.boundExtensionsPointerDown) {
1237-
this.boundExtensionsPointerDown = function (event) {
1238-
if (!self.extensionsOpen) {
1239-
return;
1240-
}
1241-
1242-
/**
1243-
* Listening on the document means `event.target` is retargeted to the
1244-
* host element; the composed path still carries the real one.
1245-
*/
1246-
var path =
1247-
typeof event.composedPath === "function" ? event.composedPath() : [];
1248-
var target = path.length > 0 ? path[0] : event.target;
1249-
1250-
if (!isInsideExtensions(target, closest)) {
1251-
self.closeExtensions(false);
1252-
}
1253-
};
1254-
1255-
document.addEventListener(
1256-
"pointerdown",
1257-
this.boundExtensionsPointerDown,
1258-
false,
1259-
);
1260-
}
12611264
};
12621265

12631266
/**

‎src/Helper/Fqcn.php‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,42 @@
77
use PHPForge\Debug\Theme\Css;
88
use UIAwesome\Html\Phrasing\{Span, Strong};
99

10+
use function preg_replace;
11+
use function sha1;
1012
use function strrpos;
13+
use function strtolower;
1114
use function substr;
15+
use function trim;
1216

1317
/**
14-
* Splits a fully-qualified class name into its short name and namespace prefix.
18+
* Derives the presentation forms of a fully-qualified class name: short name, namespace prefix, two-tone label, and
19+
* fragment anchor.
1520
*/
1621
final class Fqcn
1722
{
23+
/**
24+
* Returns a URL-safe fragment id for a fully-qualified class name, stable across spellings and free of collisions.
25+
*
26+
* The name is lowercased first because PHP resolves class names case-insensitively, so every spelling of one class
27+
* must share one anchor. The readable slug collapses each run of characters outside `a-z0-9` into a single `-`,
28+
* which on its own is not injective (`app\A_B` and `app\A__b` both flatten to `app-a-b`), so the first 8
29+
* hexadecimal characters of the `sha1` of the lowercased name are appended to keep distinct classes apart; that
30+
* digest identifies a DOM id and is never used as a security primitive. A name made only of separators leaves no
31+
* slug and returns the digest alone.
32+
*
33+
* @param string $fqcn Fully-qualified class name.
34+
*
35+
* @return string URL-safe fragment id.
36+
*/
37+
public static function anchor(string $fqcn): string
38+
{
39+
$lowercased = strtolower($fqcn);
40+
$slug = trim((string) preg_replace('/[^a-z0-9]+/', '-', $lowercased), '-');
41+
$digest = substr(sha1($lowercased), 0, 8);
42+
43+
return $slug === '' ? $digest : "{$slug}-{$digest}";
44+
}
45+
1846
/**
1947
* Returns the namespace prefix (everything before the last `\`, without trailing separator), or `''` when none is
2048
* present.

‎src/Panel/Asset/AssetPanel.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
namespace PHPForge\Debug\Panel\Asset;
66

77
use PHPForge\Debug\{ColumnStyle, Panel, PanelView, Tone};
8-
use PHPForge\Debug\Helper\{Fqcn, Text};
8+
use PHPForge\Debug\Helper\Fqcn;
99
use PHPForge\Debug\Presenter\{FileEntry, LinkInline};
1010

1111
use function array_map;
@@ -168,7 +168,7 @@ private static function card(PanelView $view, AssetBundleRow $bundle): PanelView
168168
$namespace = Fqcn::namespacePart($bundle->name);
169169

170170
return $view->card(
171-
Text::camel2id($bundle->name),
171+
Fqcn::anchor($bundle->name),
172172
AssetMessage::ID->value,
173173
Fqcn::shortName($bundle->name),
174174
$namespace === '' ? '' : "{$namespace}\\",
@@ -342,7 +342,7 @@ private static function wiring(AssetBundleRow $bundle): PanelView|null
342342
...array_map(
343343
static fn(string $depend): LinkInline => PanelView::link(
344344
Fqcn::shortName($depend),
345-
'#' . Text::camel2id($depend),
345+
'#' . Fqcn::anchor($depend),
346346
),
347347
$bundle->depends,
348348
),

‎tests/Helper/FqcnTest.php‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,73 @@
99
use PHPUnit\Framework\TestCase;
1010

1111
/**
12-
* Unit tests for {@see Fqcn} covering the shared two-tone label markup: namespace/short-name splitting, method-suffix
13-
* handling, the plain-value and empty-value fallbacks, and the `title` attribute.
12+
* Unit tests for {@see Fqcn} covering the fragment anchor and the shared two-tone label markup: namespace/short-name
13+
* splitting, method-suffix handling, the plain-value and empty-value fallbacks, and the `title` attribute.
1414
*/
1515
#[Group('helpers')]
1616
#[Group('fqcn')]
1717
final class FqcnTest extends TestCase
1818
{
19+
public function testAnchorDistinguishesNamesThatShareASlug(): void
20+
{
21+
self::assertSame(
22+
'app-a-b-e8f13edb',
23+
Fqcn::anchor('app\\A_B'),
24+
'Each separator run must collapse to one dash.',
25+
);
26+
self::assertSame(
27+
'app-a-b-91495c5b',
28+
Fqcn::anchor('app\\A__b'),
29+
'A shared slug must still carry its own digest.',
30+
);
31+
self::assertNotSame(
32+
Fqcn::anchor('app\\A_B'),
33+
Fqcn::anchor('app\\A__b'),
34+
'Distinct classes must never share an anchor.',
35+
);
36+
}
37+
38+
public function testAnchorDropsTheNamespaceForAGlobalClass(): void
39+
{
40+
self::assertSame(
41+
'globalasset-762af4c5',
42+
Fqcn::anchor('GlobalAsset'),
43+
'A class outside any namespace must slug alone.',
44+
);
45+
}
46+
47+
public function testAnchorReturnsTheDigestAloneForASeparatorOnlyName(): void
48+
{
49+
self::assertSame(
50+
'ebf29a14',
51+
Fqcn::anchor('\\_\\'),
52+
'An empty slug must leave the digest unprefixed.',
53+
);
54+
}
55+
56+
public function testAnchorSharesOneValueAcrossCaseVariantsOfOneClass(): void
57+
{
58+
self::assertSame(
59+
'app-service-mailer-c39fd4a3',
60+
Fqcn::anchor('App\\Service\\Mailer'),
61+
'The anchor must be lowercase end to end.',
62+
);
63+
self::assertSame(
64+
Fqcn::anchor('App\\Service\\Mailer'),
65+
Fqcn::anchor('app\\service\\mailer'),
66+
'Case variants of one class must share an anchor.',
67+
);
68+
}
69+
70+
public function testAnchorSlugsANamespacedClassAndAppendsItsDigest(): void
71+
{
72+
self::assertSame(
73+
'app-assets-appasset-3c6a8113',
74+
Fqcn::anchor('app\\assets\\AppAsset'),
75+
'Separators must give way to a dash-joined slug.',
76+
);
77+
}
78+
1979
public function testNamespacePartReturnsPrefixOrEmptyString(): void
2080
{
2181
self::assertSame(

‎tests/Panel/Asset/AssetPanelTest.php‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function testBundleWithoutFilesWiringOrDependenciesRendersABodylessCard()
5353
);
5454

5555
self::assertSame(
56-
'app\\assets\\-empty-asset',
56+
'app-assets-emptyasset-e3777dff',
5757
$card->id,
5858
'Anchor must survive an empty capture.',
5959
);
@@ -125,7 +125,7 @@ public function testEmptyCaptureDeactivatesThePanelAndExplainsTheMissingBundles(
125125
);
126126
}
127127

128-
public function testGlobalNamespaceBundleDropsTheSubtitleAndKeepsItsBareAnchor(): void
128+
public function testGlobalNamespaceBundleDropsTheSubtitleAndAnchorsOnItsBareClassName(): void
129129
{
130130
$card = self::card(
131131
self::blockAt(
@@ -135,9 +135,9 @@ public function testGlobalNamespaceBundleDropsTheSubtitleAndKeepsItsBareAnchor()
135135
);
136136

137137
self::assertSame(
138-
'global-asset',
138+
'globalasset-762af4c5',
139139
$card->id,
140-
'Anchor must drop to the bare class name.',
140+
'Anchor must slug the class name alone.',
141141
);
142142
self::assertSame(
143143
'GlobalAsset',
@@ -179,12 +179,23 @@ public function testRegisteredBundlesProduceTheirCardsWithFilesAndWiringColumns(
179179
self::bundle(['depends' => ['yii\\web\\YiiAsset', 'yii\\web\\JqueryAsset']]),
180180
self::bundle(
181181
[
182-
'name' => 'app\\assets\\SecondAsset',
182+
'name' => 'yii\\web\\YiiAsset',
183183
'sourcePath' => '',
184184
'basePath' => '',
185185
'baseUrl' => '',
186186
'css' => [],
187-
'js' => ['second.js'],
187+
'js' => ['yii.js'],
188+
'depends' => [],
189+
],
190+
),
191+
self::bundle(
192+
[
193+
'name' => 'yii\\web\\JqueryAsset',
194+
'sourcePath' => '',
195+
'basePath' => '',
196+
'baseUrl' => '',
197+
'css' => [],
198+
'js' => ['jquery.js'],
188199
'depends' => [],
189200
],
190201
),
@@ -197,24 +208,24 @@ public function testRegisteredBundlesProduceTheirCardsWithFilesAndWiringColumns(
197208
);
198209
self::assertEquals(
199210
[
200-
new StatEntry('asset', 'bundles', '2', Tone::MUTED),
211+
new StatEntry('asset', 'bundles', '3', Tone::MUTED),
201212
new StatEntry('brand-css3', 'css', '1', Tone::INFO),
202-
new StatEntry('brand-javascript', 'js', '3', Tone::WARNING),
213+
new StatEntry('brand-javascript', 'js', '4', Tone::WARNING),
203214
new StatEntry('link', 'links', '2', Tone::SUCCESS),
204215
],
205216
self::stats(self::blockAt($view, 0))->stats,
206217
'The aggregate tiles must total every bundle.',
207218
);
208219
self::assertEquals(
209-
[new ToolbarMetric('Bundles', '2')],
220+
[new ToolbarMetric('Bundles', '3')],
210221
$view->toolbarMetrics(),
211222
'The toolbar must report the bundle count.',
212223
);
213224

214225
$card = self::card(self::blockAt($view, 1));
215226

216227
self::assertSame(
217-
'app\\assets\\-app-asset',
228+
'app-assets-appasset-3c6a8113',
218229
$card->id,
219230
'Anchor must derive from the bundle class.',
220231
);
@@ -288,30 +299,36 @@ public function testRegisteredBundlesProduceTheirCardsWithFilesAndWiringColumns(
288299
$depends->label,
289300
'The strip must count what it lists.',
290301
);
302+
303+
$yii = self::card(self::blockAt($view, 2));
304+
$jquery = self::card(self::blockAt($view, 3));
305+
306+
self::assertSame(
307+
['yii-web-yiiasset-7afeb318', 'yii-web-jqueryasset-2772d8b9'],
308+
[$yii->id, $jquery->id],
309+
'Every dependency must own a URL-safe card anchor.',
310+
);
291311
self::assertEquals(
292312
[
293-
new LinkInline('YiiAsset', '#yii\\web\\-yii-asset', false),
294-
new LinkInline('JqueryAsset', '#yii\\web\\-jquery-asset', false),
313+
new LinkInline('YiiAsset', "#{$yii->id}", false),
314+
new LinkInline('JqueryAsset', "#{$jquery->id}", false),
295315
],
296316
$depends->links,
297317
'Each dependency must point at the anchor of its own card.',
298318
);
299-
300-
$second = self::card(self::blockAt($view, 2));
301-
302319
self::assertEquals(
303320
[new BadgeInline('1 js', Tone::WARNING)],
304-
$second->meta,
321+
$yii->meta,
305322
'An absent kind must leave no chip behind.',
306323
);
307324
self::assertCount(
308325
1,
309-
$second->columns,
326+
$yii->columns,
310327
'A bundle that publishes nothing must keep only its files.',
311328
);
312329
self::assertEquals(
313-
[new FileEntry('.js', 'second.js', Tone::WARNING)],
314-
self::files(self::columnBlockAt(self::column($second, 0), 0))->files,
330+
[new FileEntry('.js', 'yii.js', Tone::WARNING)],
331+
self::files(self::columnBlockAt(self::column($yii, 0), 0))->files,
315332
'Scripts alone must still fill the first list.',
316333
);
317334
}

‎tests/Panel/User/UserPanelTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public function testAuthenticatedIdentityDescribesItsHeroAndAttributeSections():
118118
new TextInline('secret-auth-key', TextStyle::PREVIEW),
119119
self::fields(self::overview(self::blockAt($view, 4)))['Security key']
120120
?? self::fail('The security section must keep the auth key row.'),
121-
'A sensitive attribute must stay clamped behind the standard expand control.',
121+
'The redacted key must carry the preview style.',
122122
);
123123
self::assertSame(
124124
date('M j, Y · H:i', self::TIME),

0 commit comments

Comments
 (0)