Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

get bracketarrow working again#3224

Merged
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
aklt:close-popup-3185
Jun 26, 2017
Merged

get bracketarrow working again#3224
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
aklt:close-popup-3185

Conversation

@aklt
Copy link
Copy Markdown
Contributor

@aklt aklt commented Jun 24, 2017

Associated Issue: #3109

Summary of Changes

  • Revive the small arrow pointing to the variable that the preview popup shows

Test Plan

Start debugging a file and mouse over a variable while stepping through code. The poup
has a small arrow pointing to the variable.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 24, 2017

Codecov Report

Merging #3224 into master will not change coverage.
The diff coverage is 25%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3224   +/-   ##
=======================================
  Coverage   47.71%   47.71%           
=======================================
  Files          98       98           
  Lines        4068     4068           
  Branches      836      836           
=======================================
  Hits         1941     1941           
  Misses       2127     2127
Impacted Files Coverage Δ
src/components/shared/Popover.js 5.55% <0%> (ø) ⬆️
src/components/shared/BracketArrow.js 40% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b543bc8...d5f26c6. Read the comment docs.

Copy link
Copy Markdown
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks

Comment thread src/components/shared/BracketArrow.js Outdated
""
);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting, why does this need to be a component over a function?

Comment thread src/components/shared/BracketArrow.js Outdated
left: PropTypes.number,
top: PropTypes.number,
bottom: PropTypes.number
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aklt aklt force-pushed the close-popup-3185 branch 2 times, most recently from ec600f7 to 95d4554 Compare June 25, 2017 14:16
@aklt aklt force-pushed the close-popup-3185 branch from 95d4554 to d5f26c6 Compare June 25, 2017 14:30
@aklt
Copy link
Copy Markdown
Contributor Author

aklt commented Jun 25, 2017

@jasonLaster you were right, no reason to use a class when a function suffices.

I tested in firefox 54.0 and changed the position of the arrow which was a bit off.

@jasonLaster
Copy link
Copy Markdown
Contributor

Great. Thanks for the fix

top: number,
bottom: number
) => {
}) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jasonLaster jasonLaster merged commit 04703bc into firefox-devtools:master Jun 26, 2017
@aklt aklt deleted the close-popup-3185 branch June 26, 2017 06:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants