-
-
Notifications
You must be signed in to change notification settings - Fork 489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add context menu to quickly filter bundles (resolves #241) #246
Changes from 27 commits
36acd22
8b8d64e
c6ed04e
924beec
2f93aee
34d0d8c
eadcd0c
80e51dc
60fb1a1
2d9ffad
297a946
defd41c
c63f9ca
0c9372c
31a156f
fda56e7
514561e
60354e0
7e0e87e
e39741f
65c8c03
682232f
a8fa407
64fdb24
6718ef9
c9bae5b
68b7cfb
4598e12
d9c96a3
83adbbb
2b53d43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,6 +148,22 @@ as Uglify, then this value will reflect the minified size of your code. | |
|
||
This is the size of running the parsed bundles/modules through gzip compression. | ||
|
||
<h2 align="center">Selecting Which Chunks to Display</h2> | ||
|
||
When opened, the report displays all of the Webpack chunks for your project. It's possible to filter to a more specific list of chunks by using the sidebar or the chunk context menu. | ||
|
||
### Sidebar | ||
|
||
The Sidebar Menu can be opened by clicking the `>` button at the top left of the report. You can select or deselect chunks to display under the "Show chunks" heading there. | ||
|
||
### Chunk Context Menu | ||
|
||
The Chunk Context Menu can be opened by right-clicking or `Ctrl`-clicking on a specific chunk in the report. It provides the following options: | ||
|
||
* **Hide chunk:** Hides the selected chunk | ||
* **Filter to chunk:** Hides all chunks besides the selected one | ||
This comment was marked as outdated.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
* **Show all chunks:** Un-hides any hidden chunks, returning the report to its initial, unfiltered view | ||
|
||
<h2 align="center">Troubleshooting</h2> | ||
|
||
### I can't see all the dependencies in a chunk | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
.container { | ||
font: var(--main-font); | ||
position: absolute; | ||
padding: 0; | ||
border-radius: 4px; | ||
background: #fff; | ||
border: 1px solid #aaa; | ||
list-style: none; | ||
opacity: 1; | ||
white-space: nowrap; | ||
visibility: visible; | ||
transition: opacity .2s ease, visibility .2s ease; | ||
} | ||
|
||
.hidden { | ||
opacity: 0; | ||
visibility: hidden; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/** @jsx h */ | ||
import {h} from 'preact'; | ||
import cls from 'classnames'; | ||
import ContextMenuItem from './ContextMenuItem'; | ||
import PureComponent from '../lib/PureComponent'; | ||
import {store} from '../store'; | ||
import {elementIsOutside} from '../utils'; | ||
|
||
import s from './ContextMenu.css'; | ||
|
||
export default class ContextMenu extends PureComponent { | ||
componentDidMount() { | ||
this.boundingRect = this.node.getBoundingClientRect(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the contents of the menu are static, so this optimization makes sense. It'll be necessary to update this strategy if the menu contents become dynamic enough that its sizing can change. |
||
} | ||
|
||
componentDidUpdate(prevProps) { | ||
if (this.props.visible && !prevProps.visible) { | ||
document.addEventListener('mousedown', this.handleDocumentMousedown, true); | ||
} else if (prevProps.visible && !this.props.visible) { | ||
document.removeEventListener('mousedown', this.handleDocumentMousedown, true); | ||
} | ||
} | ||
|
||
render() { | ||
const {visible} = this.props; | ||
const containerClassName = cls({ | ||
[s.container]: true, | ||
[s.hidden]: !visible | ||
}); | ||
const multipleChunksSelected = store.selectedChunks.length > 1; | ||
return ( | ||
<ul ref={this.saveNode} className={containerClassName} style={this.getStyle()}> | ||
<ContextMenuItem disabled={!multipleChunksSelected} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit of a digression, but I think it would be really great if this markup generated here was more screen reader accessible. The main challenge seems to not be in the webpack-bundle-analyzer-specific controls, but in making the whole underlying visualization accessible, such that a user could even get to a point where the controls are meaningful to use (curious if anyone does know of a common screen reader that's able to generate a meaningful result for the treemap content itself, it seems like a hard challenge). |
||
onClick={this.handleClickHideChunk}> | ||
Hide chunk | ||
</ContextMenuItem> | ||
<ContextMenuItem disabled={!multipleChunksSelected} | ||
onClick={this.handleClickFilterToChunk}> | ||
Hide all other chunks | ||
</ContextMenuItem> | ||
<hr/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might have made the laziest possible interpretation of @th0r's request in #241 (comment) to add group separators here. I could see either abstracting out the ContextMenu component so it can be passed a declarative config of options/option groups OR swapping out in favor of an existing open-source context menu, as there might be one that already has good UX thought put into it and that supports hierarchical menu options/nested menus. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No-no-no, let's keep it like that - it's absolutely fine! |
||
<ContextMenuItem disabled={store.allChunksSelected} | ||
onClick={this.handleClickShowAllChunks}> | ||
Show all chunks | ||
</ContextMenuItem> | ||
</ul> | ||
); | ||
} | ||
|
||
handleClickHideChunk = () => { | ||
const {chunk: selectedChunk} = this.props; | ||
if (selectedChunk && selectedChunk.label) { | ||
const filteredChunks = store.selectedChunks.filter(chunk => chunk.label !== selectedChunk.label); | ||
store.selectedChunks = filteredChunks; | ||
} | ||
this.hide(); | ||
} | ||
|
||
handleClickFilterToChunk = () => { | ||
const {chunk: selectedChunk} = this.props; | ||
if (selectedChunk && selectedChunk.label) { | ||
const filteredChunks = store.allChunks.filter(chunk => chunk.label === selectedChunk.label); | ||
store.selectedChunks = filteredChunks; | ||
} | ||
this.hide(); | ||
} | ||
|
||
handleClickShowAllChunks = () => { | ||
store.selectedChunks = store.allChunks; | ||
this.hide(); | ||
} | ||
|
||
handleDocumentMousedown = (e) => { | ||
if (elementIsOutside(e.target, this.node)) { | ||
e.preventDefault(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's improve UX a little bit here. Currently if menu is opened you can't trigger it on other chunk with a single right-click - you need at least two. One will close the current menu and the other will open a new one. We can do better (like native context menu in the browser) and open it in the new position on the first right-click. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! There is one remaining inconsistency, which is that the menu can be opened via a Foamtree There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can live with that. |
||
e.stopPropagation(); | ||
this.hide(); | ||
} | ||
} | ||
|
||
hide() { | ||
if (this.props.onHide) { | ||
this.props.onHide(); | ||
} | ||
} | ||
|
||
saveNode = node => (this.node = node); | ||
|
||
getStyle() { | ||
const {boundingRect} = this; | ||
|
||
// Upon the first render of this component, we don't yet know | ||
// its dimensions, so can't position it yet | ||
if (!boundingRect) return; | ||
|
||
const {coords} = this.props; | ||
|
||
const pos = { | ||
left: coords.x, | ||
top: coords.y | ||
}; | ||
|
||
if (pos.left + boundingRect.width > window.innerWidth) { | ||
// Shifting horizontally | ||
pos.left = window.innerWidth - boundingRect.width; | ||
} | ||
|
||
if (pos.top + boundingRect.height > window.innerHeight) { | ||
// Flipping vertically | ||
pos.top = coords.y - boundingRect.height; | ||
} | ||
return pos; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
.item { | ||
-webkit-user-select: none; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's better remove all the prefixed properties and create an issue to add Autoprefixer PostCSS plugin to the setup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, just removed the manual prefixing and opened an issue here: #255 |
||
-moz-user-select: none; | ||
-ms-user-select: none; | ||
-o-user-select: none; | ||
user-select: none; | ||
|
||
cursor: pointer; | ||
margin: 0; | ||
padding: 8px 14px; | ||
} | ||
|
||
.item:hover { | ||
background: #ffefd7; | ||
} | ||
|
||
.disabled { | ||
cursor: default; | ||
color: gray; | ||
} | ||
|
||
.item.disabled:hover { | ||
background: transparent; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/** @jsx h */ | ||
import {h} from 'preact'; | ||
import cls from 'classnames'; | ||
import s from './ContextMenuItem.css'; | ||
|
||
function noop() { | ||
return false; | ||
} | ||
|
||
export default function ContextMenuItem({children, disabled, onClick}) { | ||
const className = cls({ | ||
[s.item]: true, | ||
[s.disabled]: disabled | ||
}); | ||
const handler = disabled ? noop : onClick; | ||
return (<li className={className} onClick={handler}>{children}</li>); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,10 @@ export default class Treemap extends Component { | |
}, | ||
onGroupClick(event) { | ||
preventDefault(event); | ||
if ((event.ctrlKey || event.secondary) && props.onGroupSecondaryClick) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we also add
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re: Also I can document that this is a FoamTree event because that's less-than-obvious -- would you be averse to using a JsDoc-style comment for that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's up to you, but I think just a simple one-liner will work as well. |
||
props.onGroupSecondaryClick.call(component, event); | ||
return; | ||
} | ||
component.zoomOutDisabled = false; | ||
this.zoom(event.group); | ||
}, | ||
|
@@ -145,7 +149,12 @@ export default class Treemap extends Component { | |
} | ||
|
||
resize = () => { | ||
const {props} = this; | ||
this.treemap.resize(); | ||
|
||
if (props.onResize) { | ||
props.onResize(); | ||
} | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice piece of documentation 👍