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

fix: prevent multiple command palettes from being added#178

Merged
chrismwendt merged 2 commits intomasterfrom
1-hamburger
Sep 14, 2018
Merged

fix: prevent multiple command palettes from being added#178
chrismwendt merged 2 commits intomasterfrom
1-hamburger

Conversation

@chrismwendt
Copy link

This is really hacky (uses a flag to check if the palette has already been added), but it was quick to implement.

In parallel with reviews of this PR, I'll be working on a more robust solution involving each inject* function returning a Disposable/Unsubscribable that the top level can dispose of when lifecycle events occur.

Resolves #174
Resolves https://github.com/sourcegraph/sourcegraph/issues/13206

@chrismwendt chrismwendt requested review from ijsnow and sqs September 14, 2018 13:14
Copy link

@ijsnow ijsnow left a comment

Choose a reason for hiding this comment

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

LGTM


function createCommandList(): HTMLElement {
const commandListElem = document.createElement('div')
commandListElem.className = commandListClass
Copy link

Choose a reason for hiding this comment

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

Doesn't really matter here since you're creating the element from scratch but I always use element.classList.add

@chrismwendt chrismwendt merged commit 7aa19f0 into master Sep 14, 2018
@chrismwendt chrismwendt deleted the 1-hamburger branch September 14, 2018 21:20
@sourcegraph-bot
Copy link

🎉 This PR is included in version 1.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants

Comments