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

Added additional menuitems to context menu#3644

Merged
jasonLaster merged 2 commits into
firefox-devtools:masterfrom
sagorika1996:menu-items
Aug 14, 2017
Merged

Added additional menuitems to context menu#3644
jasonLaster merged 2 commits into
firefox-devtools:masterfrom
sagorika1996:menu-items

Conversation

@sagorika1996
Copy link
Copy Markdown
Contributor

Associated Issue: #3630

Summary of Changes

Added the following menuitems to breakpoint context menu:

  • Enable breakpoint
  • Disable breakpoint
  • Enable others
  • Disable others
  • Remove others
  • Enable all breakpoints
  • Disable all breakpoints

image 1

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 really great work

const enabledBreakpoints = breakpoints.filter(b => b.disabled === false);
const disabledBreakpoints = breakpoints.filter(b => b.disabled === true);
const otherEnabledBreakpoints = breakpoints.filter(
b => b.disabled === false && b !== breakpoint
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.

! b.disabled is okay too :)

};

const items = [
{ item: enableBreakpoint, hidden: () => breakpoint.disabled === false },
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.

same here

},
{
item: disableOtherBreakpoints,
hidden: () => otherEnabledBreakpoints.size === 0
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.

@clarkbw @bgrins is it better to use the hidden field or just not add it to the list? I'm guessing hidden, but not sure

);
const disableOtherBreakpointsKey = L10N.getStr(
"breakpointMenuItem.disableOthers.accesskey"
);
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.

this is the pattern we have elsewhere, but i'm wondering if we can improve upon it:

const getKey = (key) => L10N.getStr(`breakpointMenuItem.${key}.accesskey `)
const getLabel = (label) => L10N.getStr(`breakpointMenuItem.${label} `)

const foo = {
  label: getLabel("foo")
  accesskey: getKey("foo")
}

@jasonLaster
Copy link
Copy Markdown
Contributor

don't worry about the flow error - we're fixing it elsewhere

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 12, 2017

Codecov Report

Merging #3644 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3644      +/-   ##
==========================================
- Coverage   54.51%   54.39%   -0.12%     
==========================================
  Files         120      120              
  Lines        4786     4796      +10     
  Branches      992      993       +1     
==========================================
  Hits         2609     2609              
- Misses       2177     2187      +10
Impacted Files Coverage Δ
src/actions/breakpoints.js 72% <0%> (-8%) ⬇️

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 7a3af46...e964763. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 12, 2017

Codecov Report

Merging #3644 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3644      +/-   ##
=========================================
- Coverage   54.82%   54.7%   -0.12%     
=========================================
  Files         120     120              
  Lines        4810    4820      +10     
  Branches     1001    1002       +1     
=========================================
  Hits         2637    2637              
- Misses       2173    2183      +10
Impacted Files Coverage Δ
src/components/Editor/Breakpoints.js 50% <ø> (ø) ⬆️
src/reducers/pending-breakpoints.js 100% <ø> (ø) ⬆️
src/reducers/breakpoints.js 84.28% <ø> (ø) ⬆️
src/actions/breakpoints.js 72% <0%> (-8%) ⬇️

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 ce03d8f...34741fb. 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.

Thanks so much!

@jasonLaster jasonLaster merged commit e7dd7d2 into firefox-devtools:master Aug 14, 2017
@sagorika1996 sagorika1996 deleted the menu-items branch August 19, 2017 05:40
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