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

Show picker earlier with promise for progress #22

Closed
bpasero opened this issue Nov 22, 2015 · 13 comments
Closed

Show picker earlier with promise for progress #22

bpasero opened this issue Nov 22, 2015 · 13 comments
Milestone

Comments

@bpasero
Copy link
Contributor

bpasero commented Nov 22, 2015

I think it would be a nicer experience if the resolving of available yo-generators and the loading of a specific generator happens without the picker closing all the time and then reopen after a second or so.

You can always add a promise into the picker API and you will get progress while you resolve the things in the background.

@SamVerschueren
Copy link
Owner

Loading of the available generators is done with a promise but loading the questions indeed isn't if I'm not mistaken. Good suggestion!

@SamVerschueren
Copy link
Owner

It feels hacky but I don't think there is another way of doing it.

window.showQuickPick(new Promise((resolve, reject) => {
    setImmediate(() => {
        this._env.run(generator, this.done)
            .on('npmInstall', () => {
                this.setState('install node dependencies');
            })
            .on('bowerInstall', () => {
                this.setState('install bower dependencies');
            })
            .on('end', () => {
                this.clearState();
                console.log(`${EOL}${figures.tick} done`);
            });

        reject();
    });
})).then(undefined, () => { });

Maybe if there was a core method that allowed showing and hiding a progress bar?

progress

@bpasero
Copy link
Contributor Author

bpasero commented Nov 23, 2015

@SamVerschueren your solution does not look hacky to me at all you are wrapping a promise around a long running operation. What you could maybe change is to promisify more of your code, e.g. this.env.run() should already return a promise.

@SamVerschueren
Copy link
Owner

this._env is a yeoman environment, can't do anything about it I'm afraid :).

The reason I think it feels hacky is because I'm showing a quickpick component which is never used, just in order to show the progress bar...

@bpasero
Copy link
Contributor Author

bpasero commented Nov 23, 2015

Well, your entire UI is around the picker. When I start to use your extension I think it is fine to show progress via the same component that I am interacting with.

I would not like a solution where you would see some progress somewhere (e.g. status bar) where it reads "resolving available generators...", but that might be just me.

@SamVerschueren
Copy link
Owner

No, the user experience is way better now because the component is always visible. But I thought maybe there was another way of doing this. That's why I asked your opinion regarding the implementation :).

@SamVerschueren SamVerschueren added this to the v0.7.0 milestone Nov 23, 2015
@bpasero
Copy link
Contributor Author

bpasero commented Nov 23, 2015

I find it very natural to couple progress and promises together and not have an explicit way of showing progress. However, we do have some kind of progress service in the workbench that we can ask to show progress explicitly but this is currently not exposed to extensions. It would also not work very well in quick open currently. Thanks for fixing!

@SamVerschueren
Copy link
Owner

Reopening due to microsoft/vscode#693. Because the quick picker is not closed, the user will have a quick pick progress bar that just keeps spinning. That's why I reverted back this changes.

@SamVerschueren
Copy link
Owner

@bpasero just curious, will this be fixed in 0.11.0?

@bpasero
Copy link
Contributor Author

bpasero commented Dec 2, 2015

@SamVerschueren yeah although the bug is still open, the picker will close properly and you should get the error handler.

@SamVerschueren
Copy link
Owner

Awesome!

Do I have to set my engines.vscode to 0.11.x when releasing this feature? Will users of VS Code 0.10.x still be able to download the previous version with engines.vscode set to 0.10.x? Couldn't find documentation regarding this.

@bpasero
Copy link
Contributor Author

bpasero commented Dec 2, 2015

@SamVerschueren we are still discussing our version scheme, it will be well communicated once this happens!

@SamVerschueren
Copy link
Owner

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants