Skip to content
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

Merged
merged 31 commits into from Apr 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
36acd22
Expose parent asset names
bregenspan Jan 15, 2019
8b8d64e
Start adding auto-selection of bundle parents
bregenspan Jan 15, 2019
c6ed04e
Cleanup, extract function
bregenspan Jan 15, 2019
924beec
Use ctrl-click to select parent bundles
bregenspan Jan 18, 2019
2f93aee
"bundle" => "chunk"
bregenspan Jan 18, 2019
34d0d8c
Start adding context menu
bregenspan Jan 21, 2019
eadcd0c
Implement context menu options
bregenspan Jan 21, 2019
80e51dc
Basic styling + positioning
bregenspan Jan 21, 2019
60fb1a1
Fix "hide chunk" filter action
bregenspan Jan 21, 2019
2d9ffad
Hide chunk context menu after use
bregenspan Jan 21, 2019
297a946
Add "show all chunks" action
bregenspan Jan 21, 2019
defd41c
Only show hint when context menu is available
bregenspan Jan 21, 2019
c63f9ca
Lint
bregenspan Jan 21, 2019
0c9372c
Clearer isAChunk test
bregenspan Jan 21, 2019
31a156f
Hide context menu after click outside
bregenspan Jan 23, 2019
fda56e7
Styling
bregenspan Jan 23, 2019
514561e
Fix obtaining parent chunks in case of string chunk ID
bregenspan Jan 27, 2019
60354e0
Add test coverage re: exposing parentAssetNames for chunks
bregenspan Jan 28, 2019
7e0e87e
Fix flipping of menu position when on edge of viewport
bregenspan Jan 28, 2019
e39741f
Prevent treemap mousedown event triggering when closing menu
bregenspan Jan 28, 2019
65c8c03
Use elementIsOutside helper
bregenspan Jan 29, 2019
682232f
Document sidebar and context menu
bregenspan Apr 6, 2019
a8fa407
Fix flickering menu position on-close; clean up shouldRender
bregenspan Apr 7, 2019
64fdb24
Close any open context menu upon window resize
bregenspan Apr 7, 2019
6718ef9
Clearer check for differentiating assets from modules
bregenspan Apr 7, 2019
c9bae5b
Remove "filter to parent chunks" option for now
bregenspan Apr 7, 2019
68b7cfb
Disable inapplicable context menu options
bregenspan Apr 7, 2019
4598e12
Documentation: fix incorrect action name
bregenspan Apr 8, 2019
d9c96a3
Remove manually-added CSS browser prefixes
bregenspan Apr 8, 2019
83adbbb
Re-open context menu immediately when different bundle is selected
bregenspan Apr 8, 2019
2b53d43
Document event type
bregenspan Apr 8, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Expand Up @@ -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>
Copy link
Member

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 👍


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
* **Hide all other chunks:** Hides all chunks besides the selected one
* **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
Expand Down
18 changes: 18 additions & 0 deletions client/components/ContextMenu.css
@@ -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;
}
121 changes: 121 additions & 0 deletions client/components/ContextMenu.jsx
@@ -0,0 +1,121 @@
/** @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();
Copy link
Contributor Author

@bregenspan bregenspan Apr 7, 2019

Choose a reason for hiding this comment

The 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}
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I could see either abstracting out the ContextMenu component so it can be passed a declarative config of options/option groups

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();
}

/**
* Handle document-wide `mousedown` events to detect clicks
* outside the context menu.
* @param {MouseEvent} e - DOM mouse event object
* @returns {void}
*/
handleDocumentMousedown = (e) => {
const isSecondaryClick = e.ctrlKey || e.button === 2;
if (!isSecondaryClick && elementIsOutside(e.target, this.node)) {
e.preventDefault();
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 event.secondary === true event, which can include touch gestures. In such a case, the menu will still need two gestures to close and then re-open. Unfortunately, it's hard to get 1:1 parity between the handling of the Foamtree event, which doesn't expose its underlying DOM event, and this actual DOM event, and I didn't want to resort to using undocumented Foamtree methods to try to get around this.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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;
}
}
19 changes: 19 additions & 0 deletions client/components/ContextMenuItem.css
@@ -0,0 +1,19 @@
.item {
cursor: pointer;
margin: 0;
padding: 8px 14px;
user-select: none;
}

.item:hover {
background: #ffefd7;
}

.disabled {
cursor: default;
color: gray;
}

.item.disabled:hover {
background: transparent;
}
17 changes: 17 additions & 0 deletions client/components/ContextMenuItem.jsx
@@ -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>);
}
62 changes: 60 additions & 2 deletions client/components/ModulesTreemap.jsx
Expand Up @@ -11,6 +11,7 @@ import Switcher from './Switcher';
import Sidebar from './Sidebar';
import Checkbox from './Checkbox';
import CheckboxList from './CheckboxList';
import ContextMenu from './ContextMenu';

import s from './ModulesTreemap.css';
import Search from './Search';
Expand All @@ -26,13 +27,23 @@ const SIZE_SWITCH_ITEMS = [
@observer
export default class ModulesTreemap extends Component {
state = {
selectedChunk: null,
selectedMouseCoords: {x: 0, y: 0},
sidebarPinned: false,
showChunkContextMenu: false,
showTooltip: false,
tooltipContent: null
};

render() {
const {sidebarPinned, showTooltip, tooltipContent} = this.state;
const {
selectedChunk,
selectedMouseCoords,
sidebarPinned,
showChunkContextMenu,
showTooltip,
tooltipContent
} = this.state;

return (
<div className={s.container}>
Expand Down Expand Up @@ -97,10 +108,16 @@ export default class ModulesTreemap extends Component {
highlightGroups={this.highlightedModules}
weightProp={store.activeSize}
onMouseLeave={this.handleMouseLeaveTreemap}
onGroupHover={this.handleTreemapGroupHover}/>
onGroupHover={this.handleTreemapGroupHover}
onGroupSecondaryClick={this.handleTreemapGroupSecondaryClick}
onResize={this.handleResize}/>
<Tooltip visible={showTooltip}>
{tooltipContent}
</Tooltip>
<ContextMenu onHide={this.handleChunkContextMenuHide}
visible={showChunkContextMenu}
chunk={selectedChunk}
coords={selectedMouseCoords}/>
</div>
);
}
Expand Down Expand Up @@ -180,6 +197,22 @@ export default class ModulesTreemap extends Component {
store.showConcatenatedModulesContent = flag;
}

handleChunkContextMenuHide = () => {
this.setState({
showChunkContextMenu: false
});
}

handleResize = () => {
// Close any open context menu when the report is resized,
// so it doesn't show in an incorrect position
if (this.state.showChunkContextMenu) {
this.setState({
showChunkContextMenu: false
});
}
}

handleSidebarToggle = () => {
if (this.state.sidebarPinned) {
setTimeout(() => this.treemap.resize());
Expand Down Expand Up @@ -211,6 +244,25 @@ export default class ModulesTreemap extends Component {
this.setState({showTooltip: false});
};

handleTreemapGroupSecondaryClick = event => {
const {group, x, y} = event;
if (group && group.isAsset) {
this.setState({
selectedChunk: group,
selectedMouseCoords: {
x,
y
},
showChunkContextMenu: true
});
} else {
this.setState({
selectedChunk: null,
showChunkContextMenu: false
});
}
};

handleTreemapGroupHover = event => {
const {group} = event;

Expand Down Expand Up @@ -245,6 +297,12 @@ export default class ModulesTreemap extends Component {
{module.path &&
<div>Path: <strong>{module.path}</strong></div>
}
{module.isAsset &&
<div>
<br/>
<strong><em>Right-click to view options related to this chunk</em></strong>
</div>
}
</div>
);
}
Expand Down
15 changes: 15 additions & 0 deletions client/components/Treemap.jsx
Expand Up @@ -89,8 +89,18 @@ export default class Treemap extends Component {
};
}
},
/**
* Handle Foamtree's "group clicked" event
* @param {FoamtreeEvent} event - Foamtree event object
* (see https://get.carrotsearch.com/foamtree/demo/api/index.html#event-details)
* @returns {void}
*/
onGroupClick(event) {
preventDefault(event);
if ((event.ctrlKey || event.secondary) && props.onGroupSecondaryClick) {
Copy link
Collaborator

@th0r th0r Apr 8, 2019

Choose a reason for hiding this comment

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

Shall we also add event.metaKey?

I don't know anything about secondary flag. Is it being set by Preact? If not, is it cross-browser? Ah, got it - it's a FoamTree custom event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re: event.metaKey, Chrome and Firefox in OSX trigger the native context menu only when e.ctrlKey === true, not e.metaKey === true. But I should test in more browsers/OSes again and also don't see a downside to adding an extra trigger key, it could make sense to add that either way.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

would you be averse to using a JsDoc-style comment for that

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);
},
Expand Down Expand Up @@ -145,7 +155,12 @@ export default class Treemap extends Component {
}

resize = () => {
const {props} = this;
this.treemap.resize();

if (props.onResize) {
props.onResize();
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions client/utils.js
Expand Up @@ -13,3 +13,7 @@ export function walkModules(modules, cb) {
}
}
}

export function elementIsOutside(elem, container) {
return !(elem === container || container.contains(elem));
}
1 change: 1 addition & 0 deletions src/analyzer.js
Expand Up @@ -94,6 +94,7 @@ function getViewerData(bundleStats, bundleDir, opts) {
return _.transform(assets, (result, asset, filename) => {
result.push({
label: filename,
isAsset: true,
// Not using `asset.size` here provided by Webpack because it can be very confusing when `UglifyJsPlugin` is used.
// In this case all module sizes from stats file will represent unminified module sizes, but `asset.size` will
// be the size of minified bundle.
Expand Down