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

Initial UI Review #4

Closed
kcpevey opened this issue Apr 26, 2024 · 12 comments
Closed

Initial UI Review #4

kcpevey opened this issue Apr 26, 2024 · 12 comments

Comments

@kcpevey
Copy link

kcpevey commented Apr 26, 2024

Context

Moving the discussion from the initial design issue over to this repo for the review process.

image

UI nitpicks based on design:

  • wording from UI doesn't match design (e.g. "Create Empty" should be "Open New by Type" and "Launch Notebook" should be "Open new notebook by kernel")
  • Missing header "Launch New Session" at the top of the page
  • Move search to Open New By Kernel section because that is where search is most needed. Make the empty field text be "Search for Kernel."
  • Starred section is missing

Value and/or benefit

.

Anything else?

Original issue: conda-incubator/conda-store-ui#255

@krassowski
Copy link
Member

I would propose to keep the changes to headings as I did implement the design word for word initially but in my testing it took to much space and the titles were more confusing than helpful. I interpreted that part of the design to be placeholders, but if the choice of wording was deliberate we can do A/B.

The starred section should be optional, based on the discussions from design review, right? Let me add it in behind a setting.

Regarding the filter box position I think this can be configurable too because there is a trade-off: having them on each level will make it take up more space and will prevent filtering all other items. The placeholder text can be then adjusted accordingly.

@krassowski
Copy link
Member

Should we merge the "Icon" and "Kernel" columns? This would be closer to the design and I think there is not much use from having them separately. The only advantage would be that user could hide "Icon" column altogether but it does not seem very pleasant to work without icons (well maybe if all you have is Python environments only then it is not needed?)

@kcpevey
Copy link
Author

kcpevey commented Apr 29, 2024

Should we merge the "Icon" and "Kernel" columns?

Yes, I think that's a good idea. I was wondering about the verbosity of "Python [conda env:notebook]". I don't know what the other options are here, but it seems like just "python" is sufficient?

Regarding the filter box position I think this can be configurable too because there is a trade-off: having them on each level will make it take up more space and will prevent filtering all other items. The placeholder text can be then adjusted accordingly.

I dont think having the search on each level is appropriate. But right now, it was only returning options in the first section so I was confused. Maybe we should meet to discuss the behavior because I'm not sure I was getting enough context with what is available in the binder deploy.

@krassowski
Copy link
Member

I don't know what the other options are here, but it seems like just "python" is sufficient?

The format of the kernel display returned by conda_nb_kernels is customizable by CondaKernelSpecManager.name_format traitlet which goes to jupyter_server_config.py, by default '{language} [conda env:{environment}]' (docs). I think since we will be showing all details in other columns, we can configure it to just {display_name}. I am not sure if this is a responsibility of this extension; while technically it could override the server-side settings of other extensions (like conda-nb-kernels), this is a bit rude for users who want to configure it independently. Maybe we should just add a suggestion in readme.

@krassowski
Copy link
Member

krassowski commented May 6, 2024

But right now, it was only returning options in the first section so I was confused.

This is probably because there is only one environment on binder so it did not seem like it is filtering the other sections. Do you know if it is possible to deploy a conda-store with a few pre-defined environments to binder?

@krassowski
Copy link
Member

The format of the kernel display returned by conda_nb_kernels is customizable by CondaKernelSpecManager.name_format traitlet

Nebari customizes it to {environment} here

The problem with conda-store's {environment} variable is that it includes both the user name (namespace) and the environment name. We can special-case environment column to split it into two based on a known delimiter format.

@krassowski
Copy link
Member

krassowski commented May 7, 2024

Testing with nebari, here are some more tasks:

  • VS Code pretends to be a kernel, we need to special-case it to move it to "Create Empty" section
  • I think that "Notebook Jobs" do not belong in "Create Empty" section but in top-right corner with "Contextual Help"

image

@krassowski
Copy link
Member

The idea of splitting on delimiter would only work if it was not allowed in environment nor user names, but currently it is (as the delimiter is just -):

image

@krassowski
Copy link
Member

The idea of splitting on delimiter would only work if it was not allowed in environment nor user names, but currently it is (as the delimiter is just -):

Resolved with a simple regex in 68db163.

@kcpevey
Copy link
Author

kcpevey commented May 7, 2024

Do you know if it is possible to deploy a conda-store with a few pre-defined environments to binder?

I dont think its possible to deploy conda-store itself to binder, but I also don't know if its possible to deploy just multiple conda environments to binder.

Resolved with a simple regex in 68db163.

nice!

@kcpevey
Copy link
Author

kcpevey commented May 8, 2024

@krassowski should we close this issue and open more targeted issues going forward?

@krassowski
Copy link
Member

Yes, let's do that!

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

No branches or pull requests

2 participants