DevTools Link [devtools/devtools-frontend : main]
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
Kim PTAL, from implementation perspective I think this accomplishes the both cases we discussed, and also show cases them in the AI Assistance panel.
Can you review and then I can work on tests.
Related details
- Kim-Anh Tran
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {@bme...@chromium.org Should move this logic in the `UI.UIUtils.openInNewTab` helper function call together?
Related details
- Benedikt Meurer
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kim-Anh Tran (Gerrit)
Kim-Anh Tran added 7 comments![]( "Open in Gerrit")
Hi Nik! Thanks for the link, I've added a few comments 😊
class="toolbar-feedback-link devtools-link"There's a `.devtools-link` css class in `inspectorCommon.css`, the one that is used here. I think we should have the same styles in the `devtools-link`, so either we can import `inspectorCommon.css` into the component or at least use the same styles applied to it. For ensuring consistency for now, I'd say let's do the first.
"../../../core/host:bundle",Do we need this import?
// Copyright 2025 The Chromium AuthorsCould you also add a LinkImpl.docs.ts file for documentation? You can build the doc using `scripts/component_docs` as target, it creates an `index.html` in the `gen` directory which you can serve. A README.md entry on this will come soon.
export class Link extends HTMLElement {Could you please add JSDoc for this component? 😊
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {@bme...@chromium.org Should move this logic in the `UI.UIUtils.openInNewTab` helper function call together?
+1, yes please let's move this out of the UI, ideally we want to have the UI as lightweight as possible.
display: inline;As mentioned above, let's use the same styles as in `inspectorCommon.css`
Related details
- Benedikt Meurer
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Benedikt Meurer (Gerrit)
Benedikt Meurer added 2 comments![]( "Open in Gerrit")
I de
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {Kim-Anh Tran@bme...@chromium.org Should move this logic in the `UI.UIUtils.openInNewTab` helper function call together?
+1, yes please let's move this out of the UI, ideally we want to have the UI as lightweight as possible.
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {Kim-Anh Tran@bme...@chromium.org Should move this logic in the `UI.UIUtils.openInNewTab` helper function call together?
Benedikt Meurer+1, yes please let's move this out of the UI, ideally we want to have the UI as lightweight as possible.
Yep
- Benedikt Meurer
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
Do we need this import?
Done
Related details
- Benedikt Meurer
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 3 comments![]( "Open in Gerrit")
// Copyright 2025 The Chromium AuthorsCould you also add a LinkImpl.docs.ts file for documentation? You can build the doc using `scripts/component_docs` as target, it creates an `index.html` in the `gen` directory which you can serve. A README.md entry on this will come soon.
I did add it, but do I understand correctly that I need to add a new tab to the page to see the docs?
Could you please add JSDoc for this component? 😊
Done
As mentioned above, let's use the same styles as in `inspectorCommon.css`
Done
Kim-Anh Tran (Gerrit)
Kim-Anh Tran added 7 comments![]( "Open in Gerrit")
<!-- TODO: REMOVE once its show that is verified that this works -->I think we can remove this by now? 😊 I tried it out and it worked 😄
# found in the LICENSE file.super nit: please add a new line 😄
function linkExample(): HTMLElement {Can we use `lit-html` to render this instead? Since we want to switch to lit in general.
// Copyright 2025 The Chromium AuthorsNikolay VitkovCould you also add a LinkImpl.docs.ts file for documentation? You can build the doc using `scripts/component_docs` as target, it creates an `index.html` in the `gen` directory which you can serve. A README.md entry on this will come soon.
I did add it, but do I understand correctly that I need to add a new tab to the page to see the docs?
Awesome, thanks! No need, but there is the `front_end/BUILD.gn` file that need to be edited: see the [README.md](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/scripts/component_docs/README.md) for details
* @property href - The href to the place the link wants to navigateCan we add the `href` attribute too? See example with [Tooltip](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/components/tooltips/Tooltip.ts). Even if wordy, it fully documents the API.
readonly #shadow = this.attachShadow({mode: 'open'});Instead of directly calling `#render` when setting the attribute, can we switch to `observedAttributes` and `attributeChangedCallback` instead? See example in [Cards](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/components/cards/Card.ts?q=Card.ts%20devtools&ss=chromium%2Fchromium%2Fsrc)
Related details
- Benedikt Meurer
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 6 comments![]( "Open in Gerrit")
<!-- TODO: REMOVE once its show that is verified that this works -->I think we can remove this by now? 😊 I tried it out and it worked 😄
Done
super nit: please add a new line 😄
Done
Can we use `lit-html` to render this instead? Since we want to switch to lit in general.
Done
// Copyright 2025 The Chromium AuthorsNikolay VitkovCould you also add a LinkImpl.docs.ts file for documentation? You can build the doc using `scripts/component_docs` as target, it creates an `index.html` in the `gen` directory which you can serve. A README.md entry on this will come soon.
Kim-Anh TranI did add it, but do I understand correctly that I need to add a new tab to the page to see the docs?
Awesome, thanks! No need, but there is the `front_end/BUILD.gn` file that need to be edited: see the [README.md](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/scripts/component_docs/README.md) for details
Done
* @property href - The href to the place the link wants to navigateCan we add the `href` attribute too? See example with [Tooltip](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/components/tooltips/Tooltip.ts). Even if wordy, it fully documents the API.
Done
Instead of directly calling `#render` when setting the attribute, can we switch to `observedAttributes` and `attributeChangedCallback` instead? See example in [Cards](https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/components/cards/Card.ts?q=Card.ts%20devtools&ss=chromium%2Fchromium%2Fsrc)
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kim-Anh Tran (Gerrit)
Kim-Anh Tran added 5 comments![]( "Open in Gerrit")
import("../../../../scripts/build/ninja/component_doc.gni")The new line seems to be gone again. Is this the auto formatter?
* <devtools-link href=""></devtools-icon>This should be `</devtools-link>` 😄
static readonly observedAttributes = ['href', 'tabindex', 'jslogcontext'];I noticed that the casing is different here and when setting the attribute (not a big deal, as it will lowercase anyway as far as I understand, but we should keep it the same). Afterwards I was wondering about casing, and as far as I can see, attributes should be lower case and hyphened, I.e. `jslog-context` instead of `jslogcontext` [css/html guide](go/htmlcssstyle#Capitalization).
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {I just noticed now, we don't need this code block anymore, do we? Or am I missing something? Can't it be entirely replaced by `UIHelpers.openInNewTab(href)`?
/* eslint-disable @devtools/no-a-tags-in-lit */nit: formatting
Related details
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 5 comments![]( "Open in Gerrit")
import("../../../../scripts/build/ninja/component_doc.gni")The new line seems to be gone again. Is this the auto formatter?
There were two missing lines, I only added one (the one after import).
* <devtools-link href=""></devtools-icon>Nikolay VitkovThis should be `</devtools-link>` 😄
Done
static readonly observedAttributes = ['href', 'tabindex', 'jslogcontext'];I noticed that the casing is different here and when setting the attribute (not a big deal, as it will lowercase anyway as far as I understand, but we should keep it the same). Afterwards I was wondering about casing, and as far as I can see, attributes should be lower case and hyphened, I.e. `jslog-context` instead of `jslogcontext` [css/html guide](go/htmlcssstyle#Capitalization).
In some places we already use `jslogcontext` do you think the hyphened vs not is a blocker here? I will fix the casing?
if (Common.ParsedURL.schemeIs(href, 'chrome:')) {I just noticed now, we don't need this code block anymore, do we? Or am I missing something? Can't it be entirely replaced by `UIHelpers.openInNewTab(href)`?
Yes!
/* eslint-disable @devtools/no-a-tags-in-lit */- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 2 comments![]( "Open in Gerrit")
There's a `.devtools-link` css class in `inspectorCommon.css`, the one that is used here. I think we should have the same styles in the `devtools-link`, so either we can import `inspectorCommon.css` into the component or at least use the same styles applied to it. For ensuring consistency for now, I'd say let's do the first.
Done
static readonly observedAttributes = ['href', 'tabindex', 'jslogcontext'];Nikolay VitkovI noticed that the casing is different here and when setting the attribute (not a big deal, as it will lowercase anyway as far as I understand, but we should keep it the same). Afterwards I was wondering about casing, and as far as I can see, attributes should be lower case and hyphened, I.e. `jslog-context` instead of `jslogcontext` [css/html guide](go/htmlcssstyle#Capitalization).
In some places we already use `jslogcontext` do you think the hyphened vs not is a blocker here? I will fix the casing?
Done
Related details
- Kim-Anh Tran
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 2 comments![]( "Open in Gerrit")
jslogContext=${'freestyler.send-feedback'}Wront casing here.
Related details
- Kim-Anh Tran
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kim-Anh Tran (Gerrit)
Kim-Anh Tran added 1 comment![]( "Open in Gerrit")
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 2 comments![]( "Open in Gerrit")
jslogContext=${'freestyler.send-feedback'}Nikolay VitkovWront casing here.
Done
"../components/link:*",Nikolay VitkovTODO: Remove
Done
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
Related details
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
TODO: Add at least 1 test
Done
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kim-Anh Tran (Gerrit)
Kim-Anh Tran voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kim-Anh Tran
| Code-Review | +1 |
2 comments
Related details
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kim-Anh Tran (Gerrit)
Kim-Anh Tran added 1 comment![]( "Open in Gerrit")
"icons/Icon.docs.ts",You need to add the link docs here too for it to show up.
Related details
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 2 comments![]( "Open in Gerrit")
"icons/Icon.docs.ts",You need to add the link docs here too for it to show up.
Very likely lost in the rebase.
// DO We need thisNikolay Vitkovnit: leftover comment
Done
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devtools-frontend LUCI CQ (Gerrit)
Devtools-frontend LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
13 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: front_end/ui/kit/BUILD.gn
Insertions: 1, Deletions: 0.
@@ -66,6 +66,7 @@
sources = [
"cards/Card.docs.ts",
"icons/Icon.docs.ts",
+ "link/Link.docs.ts",
]
deps = [
":bundle",
```
```
The name of the file: front_end/ui/kit/link/Link.ts
Insertions: 0, Deletions: 1.
@@ -41,7 +41,6 @@
this.#setDefaultTitle();
this.setAttribute('role', 'link');
- // DO We need this
this.setAttribute('target', '_blank');
this.addEventListener('click', this.#onClick);
```
Change information
[uikit] DevTools Link
Provides a new component to link to external links, as we can't use a tags from DevTools.
This is a replacement for the XLink and should be used moving forward.
- M config/gni/devtools_grd_files.gni
- M front_end/panels/ai_assistance/AiAssistancePanel.ts
- M front_end/panels/ai_assistance/BUILD.gn
- M front_end/panels/network/components/RequestHeadersView.css
- M front_end/ui/helpers/BUILD.gn
- M front_end/ui/kit/BUILD.gn
- M front_end/ui/kit/kit.ts
- A front_end/ui/kit/link/Link.docs.ts
- A front_end/ui/kit/link/Link.test.ts
- A front_end/ui/kit/link/Link.ts
- A front_end/ui/kit/link/link.css
- M front_end/ui/visual_logging/BUILD.gn
Code-Review: +1 by Kim-Anh Tran
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |