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

Support for pinned requests #1471

Merged
merged 35 commits into from May 7, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5c50c87
Add pinning behavior
develohpanda Apr 25, 2019
c279d9d
add fa-thumb-tack
develohpanda Apr 25, 2019
4675d5b
Merged with develop
develohpanda Apr 25, 2019
dd02a96
Updated package-lock files
develohpanda Apr 26, 2019
5f9514e
Allow only top level request and request group to be pinned
develohpanda Apr 26, 2019
e4f5882
Prevent dnd for pinned request
develohpanda Apr 26, 2019
249a0fa
Efficiency change
develohpanda Apr 26, 2019
e47fe11
Reverted changes
develohpanda Apr 26, 2019
163fc7e
Revert changes to selectors.js
develohpanda Apr 26, 2019
af9075d
Fixed pin and add thumbtack
develohpanda Apr 26, 2019
3465501
Changes
develohpanda Apr 26, 2019
dba476e
Pin styling
develohpanda Apr 27, 2019
76e64a4
Fix overflow bug
develohpanda Apr 27, 2019
e53999b
styling complete
develohpanda Apr 27, 2019
a3238a6
Remove additional style
develohpanda Apr 27, 2019
2b8b950
Updated package-lock.json
develohpanda Apr 27, 2019
b8b9879
Merge branch 'develop' into feature/pinned-request
develohpanda Apr 27, 2019
ee403f9
Mergie
develohpanda Apr 27, 2019
e8079a2
Boolean to boolean
develohpanda Apr 27, 2019
aca9a95
allow nested requests to be pinned
develohpanda Apr 27, 2019
d0232fd
Remove unused arg
develohpanda Apr 27, 2019
6d0d30e
Remove folder pinning functionality
develohpanda Apr 27, 2019
174c1eb
Remove comment
develohpanda Apr 27, 2019
4684fef
Revert change to open tag on group
develohpanda Apr 30, 2019
b1da85f
Render separator with dsiplay: none so that sync menu is in correct p…
develohpanda May 4, 2019
05c8e17
Don't reset parent id on request pin
develohpanda May 4, 2019
2ce8d12
Remove extra checks
develohpanda May 4, 2019
7e6ce57
Move pin filter to selectors to prevent duplicate childTree traversal
develohpanda May 4, 2019
8938e9b
Decouple pinned items from search results
develohpanda May 4, 2019
b18447c
Hide pin on hover
develohpanda May 4, 2019
6ab94d2
Pin keyboard shortcut via shift+ctrl+p
develohpanda May 4, 2019
3fc894e
Typo fix
develohpanda May 4, 2019
278b2fb
Update mac hotkey + activeRequest nullcheck
develohpanda May 4, 2019
3461fb9
Disable drag and drop only for items in the pinned list
develohpanda May 4, 2019
cc394f8
Filter to Find
develohpanda May 6, 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
7 changes: 7 additions & 0 deletions packages/insomnia-app/app/common/hotkeys.js
Expand Up @@ -144,6 +144,8 @@ export const hotKeyRefs = {

REQUEST_SHOW_DUPLICATE: defineHotKey('request_showDuplicate', 'Duplicate Request'),

REQUEST_TOGGLE_PIN: defineHotKey('request_togglePin', 'Pin/Unpin Request'),

CLOSE_DROPDOWN: defineHotKey('closeDropdown', 'Close Dropdown'),

CLOSE_MODAL: defineHotKey('closeModal', 'Close Modal'),
Expand Down Expand Up @@ -288,6 +290,11 @@ const defaultRegistry: HotKeyRegistry = {
keyComb(true, false, false, false, keyboardKeys.d.keyCode),
),

[hotKeyRefs.REQUEST_TOGGLE_PIN.id]: keyBinds(
keyComb(false, false, false, true, keyboardKeys.p.keyCode),
Copy link
Contributor

Choose a reason for hiding this comment

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

Mac key comb should probably be false, false, true, true to map to Cmd+Shift+P because Cmd+P already maps to the request switcher. Windows/Linux one is fine though with what you have (Ctrl+Shift+P)

keyComb(true, false, true, false, keyboardKeys.p.keyCode),
),

[hotKeyRefs.CLOSE_DROPDOWN.id]: keyBinds(
keyComb(false, false, false, false, keyboardKeys.esc.keyCode),
keyComb(false, false, false, false, keyboardKeys.esc.keyCode),
Expand Down
Expand Up @@ -37,9 +37,7 @@ class RequestActionsDropdown extends PureComponent {
}

_handleSetRequestPinned() {
if (this._canPin()) {
this.props.handleSetRequestPinned(this.props.request, !this.props.isPinned);
}
this.props.handleSetRequestPinned(this.props.request, !this.props.isPinned);
}

_handleRemove() {
Expand All @@ -64,11 +62,10 @@ class RequestActionsDropdown extends PureComponent {
<DropdownButton>
<i className="fa fa-caret-down" />
</DropdownButton>
{this._canPin() && (
<DropdownItem onClick={this._handleSetRequestPinned}>
<i className="fa fa-thumb-tack" /> {this.props.isPinned ? 'Unpin' : 'Pin'}
</DropdownItem>
)}
<DropdownItem onClick={this._handleSetRequestPinned}>
<i className="fa fa-thumb-tack" /> {this.props.isPinned ? 'Unpin' : 'Pin'}
<DropdownHint keyBindings={hotKeyRegistry[hotKeyRefs.REQUEST_TOGGLE_PIN.id]} />
</DropdownItem>
<DropdownItem onClick={this._handleDuplicate}>
<i className="fa fa-copy" /> Duplicate
<DropdownHint keyBindings={hotKeyRegistry[hotKeyRefs.REQUEST_SHOW_DUPLICATE.id]} />
Expand Down Expand Up @@ -106,9 +103,7 @@ RequestActionsDropdown.propTypes = {
isPinned: PropTypes.bool.isRequired,
request: PropTypes.object.isRequired,
hotKeyRegistry: PropTypes.object.isRequired,

// Optional
handleSetRequestPinned: PropTypes.func,
handleSetRequestPinned: PropTypes.func.isRequired,
};

export default RequestActionsDropdown;
Expand Up @@ -32,6 +32,7 @@ const HOT_KEY_DEFS: Array<HotKeyDefinition> = [
hotKeyRefs.REQUEST_SHOW_DELETE,
hotKeyRefs.REQUEST_SHOW_CREATE_FOLDER,
hotKeyRefs.REQUEST_SHOW_DUPLICATE,
hotKeyRefs.REQUEST_TOGGLE_PIN,
hotKeyRefs.REQUEST_SHOW_GENERATE_CODE_EDITOR,
hotKeyRefs.SHOW_COOKIES_EDITOR,
hotKeyRefs.ENVIRONMENT_SHOW_EDITOR,
Expand Down
29 changes: 16 additions & 13 deletions packages/insomnia-app/app/ui/components/sidebar/sidebar-children.js
Expand Up @@ -16,6 +16,11 @@ type Child = {
pinned: boolean,
};

export type SidebarChildObjects = {
pinned: Array<Child>,
all: Array<Child>,
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a more appropriate place to put this type definition?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with it here. It makes sense to have it defined where it's used.

type Props = {
// Required
handleActivateRequest: Function,
Expand All @@ -29,7 +34,7 @@ type Props = {
handleGenerateCode: Function,
handleCopyAsCurl: Function,
moveDoc: Function,
childObjects: Array<Child>,
childObjects: SidebarChildObjects,
workspace: Workspace,
filter: string,
hotKeyRegistry: HotKeyRegistry,
Expand All @@ -39,7 +44,7 @@ type Props = {
};

class SidebarChildren extends React.PureComponent<Props> {
_renderChildren(children: Array<Child>): React.Node {
_renderChildren(children: Array<Child>, isInPinnedList: boolean): React.Node {
const {
filter,
handleCreateRequest,
Expand All @@ -61,15 +66,15 @@ class SidebarChildren extends React.PureComponent<Props> {
const activeRequestId = activeRequest ? activeRequest._id : 'n/a';

return children.map(child => {
if (child.hidden) {
if (!isInPinnedList && child.hidden) {
return null;
}

if (child.doc.type === models.request.type) {
return (
<SidebarRequestRow
key={child.doc._id}
filter={filter || ''}
filter={isInPinnedList ? '' : filter || ''}
moveDoc={moveDoc}
handleActivateRequest={handleActivateRequest}
handleSetRequestPinned={handleSetRequestPinned}
Expand Down Expand Up @@ -104,7 +109,7 @@ class SidebarChildren extends React.PureComponent<Props> {
}

const isActive = hasActiveChild(child.children);
const children = this._renderChildren(child.children);
const children = this._renderChildren(child.children, isInPinnedList);

return (
<SidebarRequestGroupRow
Expand All @@ -129,26 +134,24 @@ class SidebarChildren extends React.PureComponent<Props> {
});
}

_renderList(children: Array<Child>): React.Node {
_renderList(children: Array<Child>, pinnedList: boolean): React.Node {
return (
<ul className="sidebar__list sidebar__list-root theme--sidebar__list">
{this._renderChildren(children)}
{this._renderChildren(children, pinnedList)}
</ul>
);
}

render() {
const { childObjects } = this.props;

const pinnedChildren = childObjects.filter(c => c.pinned);
const unpinnedChildren = childObjects.filter(c => !c.pinned);
const showSeparator = pinnedChildren.length > 0 && unpinnedChildren.length > 0;
const showSeparator = childObjects.pinned.length > 0;

return (
<React.Fragment>
{this._renderList(pinnedChildren)}
{showSeparator && <div className="sidebar__list-separator" />}
{this._renderList(unpinnedChildren)}
{this._renderList(childObjects.pinned, true)}
<div className={`sidebar__list-separator${showSeparator ? '' : '--invisible'}`} />
{this._renderList(childObjects.all, false)}
</React.Fragment>
);
}
Expand Down
Expand Up @@ -125,7 +125,7 @@ class SidebarRequestRow extends PureComponent {
<Highlight search={filter} text={value} {...props} />
)}
/>
{isPinned && <i className="sidebar__item__icon-x fa fa-thumb-tack" />}
{isPinned && <i className="sidebar__item__icon-pin fa fa-thumb-tack" />}
</div>
</button>
<div className="sidebar__actions">
Expand Down
2 changes: 1 addition & 1 deletion packages/insomnia-app/app/ui/components/sidebar/sidebar.js
Expand Up @@ -178,7 +178,7 @@ Sidebar.propTypes = {
width: PropTypes.number.isRequired,
isLoading: PropTypes.bool.isRequired,
workspace: PropTypes.object.isRequired,
childObjects: PropTypes.arrayOf(PropTypes.object).isRequired,
childObjects: PropTypes.object.isRequired,
workspaces: PropTypes.arrayOf(PropTypes.object).isRequired,
unseenWorkspaces: PropTypes.arrayOf(PropTypes.object).isRequired,
environments: PropTypes.arrayOf(PropTypes.object).isRequired,
Expand Down
5 changes: 3 additions & 2 deletions packages/insomnia-app/app/ui/components/wrapper.js
Expand Up @@ -10,6 +10,7 @@ import type {
RequestHeader,
RequestParameter,
} from '../../models/request';
import type { SidebarChildObjects } from './sidebar/sidebar-children';

import * as React from 'react';
import autobind from 'autobind-decorator';
Expand Down Expand Up @@ -115,7 +116,7 @@ type Props = {
sidebarWidth: number,
sidebarHidden: boolean,
sidebarFilter: string,
sidebarChildren: Array<Object>,
sidebarChildren: SidebarChildObjects,
settings: Settings,
workspaces: Array<Workspace>,
unseenWorkspaces: Array<Workspace>,
Expand Down Expand Up @@ -616,7 +617,7 @@ class Wrapper extends React.PureComponent<Props, State> {
<AddKeyCombinationModal ref={registerModal} />
<ExportRequestsModal
ref={registerModal}
childObjects={sidebarChildren}
childObjects={sidebarChildren.all}
handleExportRequestsToFile={handleExportRequestsToFile}
/>
</ErrorBoundary>
Expand Down
13 changes: 12 additions & 1 deletion packages/insomnia-app/app/ui/containers/app.js
Expand Up @@ -215,6 +215,18 @@ class App extends PureComponent {
await this._requestDuplicate(this.props.activeRequest);
},
],
[
hotKeyRefs.REQUEST_TOGGLE_PIN,
async () => {
const metas = Object.values(this.props.entities.requestMetas).filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be using .find() instead of .filter()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL. Will update this evening!

m => m.parentId === this.props.activeRequest._id,
);
await this._handleSetRequestPinned(
this.props.activeRequest,
!(metas[0] && metas[0].pinned),
);
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a big fan of this solution, but the other solution would be to put the pinned on Request rather than RequestMeta, but more changes will need to go into app.js to support it. 🤔

Are there any other options that I am missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just thinking, whatever solution is decided here could also be used to show a pin icon/reordering for pinned requests in the quick switcher.

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to keep pinned on RequestMeta. The "meta" models are meant to store information that isn't related to actually sending the request and also that should not be synced or shared.

Note. One thing that would help here is using the getOrCreateByParentId method on request-meta.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you'll need to add a check to make sure activeRequest is not null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note. One thing that would help here is using the getOrCreateByParentId method on request-meta.js.

_handleSetRequestPinned internally calls App._updateRequestMetaByParentId, which is the same one used by the other handlers and supports getOrCreate. If it is a new request (with no meta) then this handler also creates meta.

https://github.com/getinsomnia/insomnia/blob/d8066bc55887b865c359a8f8815c44c9f7bdf325/packages/insomnia-app/app/ui/containers/app.js#L388-L396

Copy link
Contributor

@gschier gschier May 5, 2019

Choose a reason for hiding this comment

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

Ya I saw that. I thought your first comment was about the complexity of finding the request meta in the entities list.

I was suggesting using getOrCreate instead of having to write the manual filter on the entities list

],
[hotKeyRefs.SIDEBAR_TOGGLE, this._handleToggleSidebar],
[hotKeyRefs.PLUGIN_RELOAD, this._handleReloadPlugins],
[
Expand Down Expand Up @@ -439,7 +451,6 @@ class App extends PureComponent {

async _handleSetRequestPinned(request, pinned) {
App._updateRequestMetaByParentId(request._id, { pinned });
models.request.update(request, { parentId: this.props.activeWorkspace._id });
}

_handleSetResponsePreviewMode(requestId, previewMode) {
Expand Down
13 changes: 11 additions & 2 deletions packages/insomnia-app/app/ui/css/components/sidebar.less
Expand Up @@ -193,6 +193,10 @@
height: 2px;
background-color: var(--hl-sm);
margin: @padding-sm 0;

&--invisible {
display: none;
}
}

.sidebar__list--collapsed > * {
Expand Down Expand Up @@ -281,8 +285,9 @@
padding-right: @padding-sm;
}

&-x {
&-pin {
padding: 0 @padding-sm;
display: initial;
}
}

Expand All @@ -292,7 +297,7 @@

.sidebar__clickable {
display: grid;
grid-template-columns: auto minmax(0, 1fr) auto auto;
grid-template-columns: auto minmax(0, 1fr) auto;
align-items: center;
height: 100%;

Expand Down Expand Up @@ -389,6 +394,10 @@
display: initial;
}

.sidebar__item:hover .sidebar__item__icon-pin {
display: none;
}

// ~~~~~~~~~~~~~~ //
// Sidebar Footer //
// ~~~~~~~~~~~~~~ //
Expand Down
31 changes: 21 additions & 10 deletions packages/insomnia-app/app/ui/redux/selectors.js
Expand Up @@ -135,7 +135,7 @@ export const selectSidebarChildren = createSelector(
(collapsed, pinned, activeWorkspace, activeWorkspaceMeta, childrenMap) => {
const sidebarFilter = activeWorkspaceMeta ? activeWorkspaceMeta.sidebarFilter : '';

function next(parentId) {
function next(parentId, pinnedChildren) {
const children = (childrenMap[parentId] || [])
.filter(doc => {
return doc.type === models.request.type || doc.type === models.requestGroup.type;
Expand All @@ -149,15 +149,23 @@ export const selectSidebarChildren = createSelector(
});

if (children.length > 0) {
return children.map(c => ({
doc: c,
hidden: false,
collapsed: !!collapsed[c._id],
pinned: !!pinned[c._id],
return children.map(c => {
const child = {
doc: c,
hidden: false,
collapsed: !!collapsed[c._id],
pinned: !!pinned[c._id],
};

if (child.pinned) {
pinnedChildren.push(child);
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 did this for efficiency. Extracting all pinned requests (ie. nested requests) in sidebar-children would require an extra unnecessary traversal of childrenTree.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it actually fits well here within the context of "sidebar children" since the only place it's used is in the sidebar. If/when pinned requests are needed elsewhere, this would make less sense. For now, it's good though.

}

// Don't add children of requests
children: c.type === models.request.type ? [] : next(c._id),
}));
child.children = c.type === models.request.type ? [] : next(c._id, pinnedChildren);

return child;
});
} else {
return children;
}
Expand Down Expand Up @@ -189,8 +197,11 @@ export const selectSidebarChildren = createSelector(
return children;
}

const childrenTree = next(activeWorkspace._id, false);
return matchChildren(childrenTree);
let pinnedChildren = [];
const childrenTree = next(activeWorkspace._id, pinnedChildren);
const matchedChildren = matchChildren(childrenTree);

return { pinned: pinnedChildren, all: matchedChildren };
},
);

Expand Down