Skip to content

Commit 3872832

Browse files
AlinaVarkkiDevtools-frontend LUCI CQ
authored andcommitted
[RPP][AI] Remove button flickering and fix button visibility bug
Only animating the width gets rid of the flickering. Also fix the button not disappearing the the label is not empty. Before: https://screencast.googleplex.com/cast/NDcxMjU2ODAzNDY4OTAyNHw0MmRjMjdhMi03MA After: http://screencast/cast/NjI2NTUzNTgwMDAxNjg5Nnw3Nzg3M2Y3Yy02Mg Bug: 405980546 Change-Id: I7fbf4a57cf1562ada7a708627a877b61c688cb1e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6387104 Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Jack Franklin <[email protected]> Auto-Submit: Alina Varkki <[email protected]>
1 parent 2052db6 commit 3872832

File tree

3 files changed

+60
-17
lines changed

3 files changed

+60
-17
lines changed

front_end/panels/timeline/overlays/OverlaysImpl.test.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ describeWithEnvironment('Overlays', () => {
649649
);
650650
});
651651

652-
it('Shows disabled `generate ai label` button if the user is not logged into their google account or is under 18',
652+
it('Does not show `generate ai label` button if the label is not empty',
653653
async function() {
654654
updateHostConfig({
655655
aidaAvailability: {
@@ -666,6 +666,48 @@ describeWithEnvironment('Overlays', () => {
666666
await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'entry label');
667667
assert.strictEqual(inputField?.innerText, 'entry label');
668668

669+
const aiLabelButtonWrapper =
670+
elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;
671+
// Button should not exist
672+
assert.isNotOk(aiLabelButtonWrapper);
673+
});
674+
675+
it('Shows the `generate ai label` button if the label is empty', async function() {
676+
updateHostConfig({
677+
aidaAvailability: {
678+
enabled: false,
679+
blockedByAge: true,
680+
blockedByEnterprisePolicy: false,
681+
blockedByGeo: false,
682+
disallowLogging: true,
683+
enterprisePolicyValue: 1,
684+
},
685+
});
686+
687+
const {elementsWrapper, inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, '');
688+
assert.strictEqual(inputField?.innerText, '');
689+
690+
const aiLabelButtonWrapper =
691+
elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;
692+
assert.isOk(aiLabelButtonWrapper);
693+
});
694+
695+
it('Shows disabled `generate ai label` button if the user is not logged into their google account or is under 18',
696+
async function() {
697+
updateHostConfig({
698+
aidaAvailability: {
699+
enabled: false,
700+
blockedByAge: true,
701+
blockedByEnterprisePolicy: false,
702+
blockedByGeo: false,
703+
disallowLogging: true,
704+
enterprisePolicyValue: 1,
705+
},
706+
});
707+
708+
const {elementsWrapper, inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, '');
709+
assert.strictEqual(inputField?.innerText, '');
710+
669711
const aiLabelButtonWrapper =
670712
elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;
671713
assert.isOk(aiLabelButtonWrapper);
@@ -690,9 +732,8 @@ describeWithEnvironment('Overlays', () => {
690732
},
691733
});
692734

693-
const {elementsWrapper, inputField} =
694-
await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'entry label');
695-
assert.strictEqual(inputField?.innerText, 'entry label');
735+
const {elementsWrapper, inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, '');
736+
assert.strictEqual(inputField?.innerText, '');
696737

697738
const aiLabelButtonWrapper =
698739
elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;

front_end/panels/timeline/overlays/components/EntryLabelOverlay.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -569,15 +569,6 @@ export class EntryLabelOverlay extends HTMLElement {
569569
const noLogging = Root.Runtime.hostConfig.aidaAvailability?.enterprisePolicyValue ===
570570
Root.Runtime.GenAiEnterprisePolicyValue.ALLOW_WITHOUT_LOGGING;
571571

572-
/**
573-
* Right now if the user "retries" the AI label generation the result will
574-
* be almost identical because we don't change the input data or prompt. So
575-
* we only show the generate button if the label is empty.
576-
*/
577-
if (this.#label.length > 0) {
578-
return Lit.nothing;
579-
}
580-
581572
// clang-format off
582573
return html`
583574
<!-- 'preventDefault' on the AI label button to prevent the label removal on blur -->
@@ -652,8 +643,14 @@ export class EntryLabelOverlay extends HTMLElement {
652643
const hasAiExperiment = Boolean(Root.Runtime.hostConfig.devToolsAiGeneratedTimelineLabels?.enabled);
653644
const aiDisabledByEnterprisePolicy = Root.Runtime.hostConfig.aidaAvailability?.enterprisePolicyValue ===
654645
Root.Runtime.GenAiEnterprisePolicyValue.DISABLE;
646+
/**
647+
* Right now if the user "retries" the AI label generation the result will
648+
* be almost identical because we don't change the input data or prompt. So
649+
* we only show the generate button if the label is empty.
650+
*/
651+
const labelIsEmpty = this.#label?.length <= 0;
655652

656-
const doNotShowAIButton = !hasAiExperiment || aiDisabledByEnterprisePolicy;
653+
const doNotShowAIButton = !hasAiExperiment || aiDisabledByEnterprisePolicy || !labelIsEmpty;
657654
// clang-format off
658655
Lit.render(
659656
html`
@@ -667,7 +664,12 @@ export class EntryLabelOverlay extends HTMLElement {
667664
@blur=${() => this.setLabelEditabilityAndRemoveEmptyLabel(false)}
668665
@keydown=${this.#handleLabelInputKeyDown}
669666
@paste=${this.#handleLabelInputPaste}
670-
@keyup=${this.#handleLabelInputKeyUp}
667+
@keyup=${() => {
668+
this.#handleLabelInputKeyUp();
669+
// Rerender the label component when the label text changes because we need to
670+
// make sure the 'auto annotation' button is only shown when the label is empty.
671+
this.#render();
672+
}}
671673
contenteditable=${this.#isLabelEditable ? 'plaintext-only' : false}
672674
jslog=${VisualLogging.textField('timeline.annotations.entry-label-input').track({keydown: true, click: true})}
673675
></span>

front_end/panels/timeline/overlays/components/entryLabelOverlay.css

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
align-items: center;
3737
gap: var(--sys-size-4);
3838
pointer-events: auto;
39+
transition:
40+
width var(--sys-motion-duration-long2) var(--sys-motion-easing-emphasized);
3941

4042
* {
4143
/* Make up for the padding in a hovered button to only see the pen icon when it is not hovered */
@@ -48,8 +50,6 @@
4850
width: fit-content;
4951
height: var(--sys-size-13);
5052
padding: var(--sys-size-3) var(--sys-size-5);
51-
transition:
52-
all var(--sys-motion-duration-long2) var(--sys-motion-easing-emphasized);
5353
overflow: hidden;
5454
top: -4px;
5555

0 commit comments

Comments
 (0)