Skip to content

Conversation

@koaning
Copy link
Contributor

@koaning koaning commented Jul 21, 2025

It felt like these two were the main culprits, at least in my workflow.

I was contemplating to change this code:

dropdown: PANELS.flatMap(({ type, Icon, hidden }) => {
  if (hidden) {
    return [];
  }
  return {
    label: startCase(type), // <-- label is generated here
    rightElement: renderCheckboxElement(selectedPanel === type),
    icon: <Icon size={14} strokeWidth={1.5} />,
    handle: () => toggleApplication(type),
  };
}),

Maybe we could use the tooltip instead of the type to render the label? But I worried that by changing this that I might break existing workflows of folks. So maybe just adding these two options is best.

@koaning koaning requested review from Light2Dark and manzt as code owners July 21, 2025 12:23
@vercel
Copy link

vercel bot commented Jul 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2025 2:13pm

@koaning koaning changed the title More command keywords WIP: More command keywords Jul 21, 2025
@koaning koaning changed the title WIP: More command keywords More command keywords Jul 21, 2025
@Light2Dark
Copy link
Contributor

Using the tooltip looks okay to me imo and is more descriptive in the command pallete. We shouldn't add another command when it already exists.

CleanShot 2025-07-21 at 22 16 17@2x

Another option is to add an optional label param and add it for just these 2 panels.

@manzt
Copy link
Contributor

manzt commented Jul 21, 2025

I agree with @Light2Dark, think the tooltip would be fine to use as a label. I think we have more freedom to change these kind of "on-demand" behaviors in the UI.

May I suggest we use a more descriptive PR title like "Add additional command palette actions for documentation and AI chat panels"? The PR titles are copied into release notes so it would be useful to help discoverability by end users of these new additions.

@koaning koaning changed the title More command keywords Add additional command palette actions for documentation and AI chat panels Jul 22, 2025
Light2Dark
Light2Dark previously approved these changes Jul 26, 2025
@Light2Dark Light2Dark added the documentation Improvements or additions to documentation label Jul 26, 2025
Copy link
Contributor

@mscolnick mscolnick left a comment

Choose a reason for hiding this comment

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

i think we can add something call extraDescription that is just used for searching. using tooltip as the label will may lead to some other side effects

@koaning
Copy link
Contributor Author

koaning commented Jul 29, 2025

I added a key extraDescription per comment from @mscolnick that also lets us add extra wording. In the case of the documentation panel, just to name one example, we're also able to refer to terms that people may remember from Jupyter like "contextual helper". It's the same feature, but in Juypter it has another name.

label: "Helper panel",
handle: NOOP_HANDLER,
dropdown: PANELS.flatMap(({ type, Icon, hidden }) => {
dropdown: PANELS.flatMap(({ type, Icon, hidden, extraDescription }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is going to create a lot of dropdowns (which we don't want). we should instead fire through a custom filter prop to in frontend/src/components/editor/controls/command-palette.tsx to <CommandDialog that uses these extra descriptions

Copy link
Contributor Author

@koaning koaning Jul 30, 2025

Choose a reason for hiding this comment

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

I feel inclined to push back a little bit, but this might just be a lack of typescript/react knowledge on my part, so feel free to push back too.

My concern is that if a user types "contextual helper" and they see "documentation" that they may experience that as a "oh, i guess this feature is not here". That's the thing I really want to prevent. I think the only way to get there is to have extra dropdown items because if we just show the "documentation" element without any matching terms the user may still get confused.

Copy link
Contributor

Choose a reason for hiding this comment

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

the problem is, this is what your change is doing

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Now I understand!

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants