Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

introduce numberOfRecentlyConfirmedCommandsShowsAtTop #103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t9md
Copy link
Contributor

@t9md t9md commented Dec 5, 2017

Description of the Change

Fix #18

There is already PR #86, #87.
This PR somewhat takeover of #86.

Introduce new configuration numberOfRecentlyConfirmedCommandsShowsAtTop(default 0).
When set to greater than 0, it shows specified number of recently confirmed commands at top of list.
So user can simply confirm last confirmed item by "cmd-shift-p then enter".

Why I send this PR when there is already similar PRs in the queue is, I think this PR's implementation is more straightforward in implementation(just move recent items to top of command array on show() timing).

Why invalidation of cached element is important

If we return cached element for item which is moved to top of list, it breaks at here.

https://github.com/atom/atom-select-list/blob/ee325e75833acc84af9f9c90aed30157e1816f36/src/select-list-view.js#L449

atom-select-list#ListItemView::update() does directly manipulate DOM element by replaceChild which assume this.element is stable at position in <ol>.

Alternate Designs

#86, #87

Benefits

Described in #18.
When user want to invoke last confirmed again, it's possible by just "cmd-shift-p then enter" workflow.

Possible Drawbacks

None.

Applicable Issues

#18

t9md added 2 commits December 6, 2017 05:29
numberOfRecentlyConfirmedCommandsShowsAtTop(default 0)
Which shows recently confirmed commands to top of commands list
@loot-king
Copy link

🥺```

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.

Show recently used commands [enhancement]
2 participants