Replace `defer` usage with `async/await` in devtools/client/shared/widgets/tooltip/SwatchCubicBezierTooltip.js

RESOLVED FIXED in Firefox 67

Status

enhancement
P3
normal
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: nchevobbe, Assigned: paarmita1998, Mentored)

Tracking

(Blocks 1 bug, {good-first-bug})

Trunk
Firefox 67
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

4 months ago

defer is used in devtools/client/shared/widgets/tooltip/SwatchCubicBezierTooltip.js#52-61, but we should use new Promise instead.

Here's the current code:

  const def = defer();
  // Wait for the tooltip to be shown before calling instanciating the widget
  // as it expect its DOM elements to be visible.
  this.tooltip.once("shown", () => {
    const widget = new CubicBezierWidget(container, bezier);
    def.resolve(widget);
  });
  return def.promise;

What we want to have instead is transform the function to an async one, and in the code await on the tooltip:

  // Wait for the tooltip to be shown before calling instanciating the widget
  // as it expect its DOM elements to be visible.
  await this.tooltip.once("shown");
  return new CubicBezierWidget(container, bezier);
Reporter

Updated

4 months ago
Mentor: nchevobbe

Comment 1

4 months ago

Hi,i am an outreachy applicant and am looking to make my contribution.could you assign it to me so that i can start working on it. Thanks

Reporter

Comment 2

4 months ago

Hello again asish.
I just assigned you another bug, so maybe we should wait until you are done with it? (also we may want to keep this one for the first bug of another person maybe)

Assignee

Comment 3

4 months ago

Hey,
I am also an outreachy applicant and would like to take this up as my first contribution?
Can you please assign it to me?

Reporter

Comment 4

4 months ago

Hello paarmita1998, thanks for wanting to help us!

I assigned the bug to you, so you can start working on it 🙂
You can read http://docs.firefox-dev.tools/getting-started/ to setup the work environment. Make sure to use Artifact Builds when asked to as it's much faster.
Feel free to ask any question, either here or on our Slack.

Assignee: nobody → paarmita1998
Component: Console → Shared Components
Reporter

Updated

4 months ago
Status: NEW → ASSIGNED

(In reply to Nicolas Chevobbe from comment #0)

defer is used in devtools/client/shared/widgets/tooltip/SwatchCubicBezierTooltip.js#52-61, but we should use new Promise instead.

Here's the current code:

  const def = defer();
  // Wait for the tooltip to be shown before calling instanciating the widget
  // as it expect its DOM elements to be visible.
  this.tooltip.once("shown", () => {
    const widget = new CubicBezierWidget(container, bezier);
    def.resolve(widget);
  });
  return def.promise;

What we want to have instead is transform the function to an async one, and in the code await on the tooltip:

  // Wait for the tooltip to be shown before calling instanciating the widget
  // as it expect its DOM elements to be visible.
  await this.tooltip.once("shown");
  return new CubicBezierWidget(container, bezier);

Hello, I would like to ask if I can be assigned to this bug if no one is currently working on it. Thanks.

Assignee

Comment 6

4 months ago

Hey Erik, I am working on it.

Reporter

Comment 7

4 months ago

Erik, make sure to check the bug status when commenting :)

Assignee

Comment 8

4 months ago

Hey Nicolas, Can you please review the patch- https://phabricator.services.mozilla.com/D21406

Attachment #9047138 - Attachment description: Bug 1530275- Replace defer usage with async/await → Bug 1530275- Replace defer usage with async/await; r=nchevobbe.
Attachment #9047138 - Attachment description: Bug 1530275- Replace defer usage with async/await; r=nchevobbe. → Bug 1530275- Replace defer usage with async/await in SwatchCubicBezierTooltip.js; r=nchevobbe.
Reporter

Comment 10

4 months ago
Posted image image.png

I realize I didn't wrote a proper explanation on how to show the widget, to ensure the changes don't break anything.

So, to show the widget, you should:

  1. In the URL bar, paste data:text/html,<meta charset=utf8><style>h1 { color: red; transition: color 0.3s ease-in; } h1:hover {color: yellow;}</style><h1>Hello</h1> . That will open a simple page with an animation on hover.
  2. Right click on "Hello", and select "Inspect the element" to open the inspector.
  3. In the Inspector rule view, in the transition line, you should see an icon before "ease-in", click on it
  4. This should open the widget, and it should look like what's in the attachment.

Make sure you can reproduce all these steps before submitting a new patch :)

Assignee

Comment 11

4 months ago

Okay, will do it! Thanks.

Assignee

Comment 12

4 months ago

Hey I followed the above steps and made the new patch
Can you please review- https://phabricator.services.mozilla.com/D21591

Attachment #9047138 - Attachment description: Bug 1530275- Replace defer usage with async/await in SwatchCubicBezierTooltip.js; r=nchevobbe. → Bug 1530275 - Replace defer usage with async/await in SwatchCubicBezierTooltip.js; r=nchevobbe.
Attachment #9047138 - Attachment is obsolete: true
Attachment #9047508 - Attachment is obsolete: true

Comment 15

4 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ccb0b59aa78e
Replace defer usage with async/await in SwatchCubicBezierTooltip.js; r=nchevobbe

Comment 16

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.