Add initial Command Pallette addon #449

ericholscher · 2024-11-23T23:04:14Z

This is one I'm pretty excited about.
It takes the Command Pallette pattern from Slack, GitHub, etc.,
and adds it to any documentation page.

I think this could be an interesting UX for our future UX,
and enable a lot of really interesting power user features.

The really nice thing is that it's totally based on a keyboard shortcut,
so doesn't require any UX, and generally folks won't want to disable it.

Demo

Screen.Recording.2024-11-23.at.3.12.46.PM.mov

Requires readthedocs/readthedocs.org#11791

This is one I'm pretty excited about. It takes the Command Pallette pattern from Slack, GitHub, etc., and adds it to any documentation page. I think this could be an interesting UX for our future UX, and enable a lot of really interesting power user features. The really nice thing is that it's totally based on a keyboard shortcut, so doesn't require any UX, and generally folks won't want to disable it.

humitos · 2024-11-25T11:44:09Z

I think this is an interesting idea to explore more 👍🏼 . It seems we don't need to create a different addon and UI for this and it can probably be integrated with the search addon -- like a feature of the search addon: you type anything and you see the results for the "commands" and for the "search" in the same UI.

ericholscher · 2024-11-25T16:50:52Z

@humitos Yea, I think the main issue I have with search is just the UX. The really nice thing about the filtering UI is that it's instant, whereas search has a pretty long lag. It's going to be tricky to figure out how to build that UX, but I was wondering if page titles and section titles are in-memory matched, but then page results are queried against ES. I think we could build a reasonable UX there, but it's gonna take a little thought and discussion.

I think there's a huge win of the instant feel of the UX of the filtering UI though, and I really want to make sure we keep that working.

A lot of programs have a similar concept where search and filtering are different UX, for example GitHub and VS Code. So I don't think it's obviously wrong to have 2 different UIs. We could ship a default "Command" that is Search $query that shows at the bottom, for example.

ericholscher · 2024-12-02T17:18:08Z

@humitos any additional thoughts here ^^

I still think the Command Pallette is a powerful UX for exposing a bunch more functionality to users, and would love to get at least a internal-only test going at some point soonish.

humitos

I did a code review and I left a few comments and suggestions.

I think this is an interesting concept. However, I'd say we should work on other pieces before moving forward with this (eg. backend API for .json file, add required AddonsConfig fields, etc).

This implementation copies a lot of the search code/UI which, unfortunately, is the least polished one. I mentioned in another comment that I'd like to see the Command Palette and Search UI combined, making these command items inside the same UI.

I don't think this is ready to be exposed to users yet, but we can merge it for internal-only testing once we have done the backend work if you want.

humitos · 2024-12-03T08:52:42Z

src/commands.js

+    this.commands = [];
+    this.inputIcon = icon(faMagnifyingGlass, { title: "Search commands" });
+    this.currentQueryRequest = null;
+    this.triggerKeycode = 191;


This will collide with the hotkey for search.

humitos · 2024-12-03T08:54:03Z

src/commands.js

+   *   sections: {
+   *   id: string, // The unique identifier of the section
+   *   title: string, // The title of the section
+   *   }


What a "section" means in the context of a command?

humitos · 2024-12-03T08:55:53Z

src/commands.js

+      },
+    ];
+
+    if (config.projects && config.projects.current) {


There is no need to check for all of these fields. These are always coming in the API response. You just need to add them to the data validation as required fields. The same happens with config.builds below.

humitos · 2024-12-03T08:58:10Z

src/commands.js

+      config.addons.filesections &&
+      config.addons.filesections.pages


This should a URL with the manifest instead and the client should retrieve and parse this data. I mentioned this in the backend PR: readthedocs/readthedocs.org#11791 (comment)

humitos · 2024-12-03T08:58:32Z

src/commands.js

+  renderCommandPalette() {
+    console.log("Rendering command palette modal");
+    return html`
+      <div ?hidden=${!this.show} role="search">


This shouldn't be search role.

Note there are more references to search in other places as well.

humitos · 2024-12-03T09:08:08Z

src/data-validation.js

+  properties: {
+    addons: {
+      type: "object",
+      required: ["filesections"],


This shouldn't be required since projects without filesections won't have access to the commands palette.

humitos · 2024-12-03T09:09:01Z

src/data-validation.js

+  $id: "http://v1.schemas.readthedocs.org/addons.commands.json",
+  type: "object",
+  required: ["addons"],
+  properties: {


Here you want to have projects, versions and builds required as well.

humitos · 2024-12-03T09:09:12Z

src/data-validation.js

+// Validator for FileSections Addon
+const addons_filesections = {


This should make reference to commands, instead of filesections.

humitos · 2024-12-03T09:10:33Z

src/hotkeys.js

@@ -43,6 +44,7 @@ export class HotKeysElement extends LitElement {
    this.docDiffShowed = false;

    this.searchHotKeyEnabled = this.config.addons.hotkeys.search.enabled;
+    this.commandPaletteHotKeyEnabled = true;


We should decide for one name and stick with it everywhere in the code; variables, functions and event names: Command or CommandPalette.

humitos · 2024-12-03T09:11:56Z

src/hotkeys.js

+      (e.metaKey || e.ctrlKey) &&
+      e.key === "k" &&


This hotkey combination should come from the API.

ericholscher · 2024-12-03T21:05:51Z

This implementation copies a lot of the search code/UI which, unfortunately, is the least polished one. I mentioned in another comment that I'd like to see the Command Palette and Search UI combined, making these command items inside the same UI.

This is what I was asking for feedback on, was my discussion about this above: #449 (comment)

In particular, how does it make sense to combine things that are realtime lookups with search lookups that are delayed. I think that's going to be a much harder UX challenge.

I do think the Command Palette concept could be a pretty powerful thing to expose, and give us a bunch more functionality that we can add outside of the Flyout, but mostly for advanced users. I think if we shipped it, I would use it on our docs everyday tho, which I think is a pretty powerful vote for it being something powerful.

humitos · 2024-12-04T12:52:33Z

In particular, how does it make sense to combine things that are realtime lookups with search lookups that are delayed. I think that's going to be a much harder UX challenge.

We can show the results from commands immediately at the top, and below them in another section, the results that comes from the search API. Long search queries won't have any result from commands so that section won't be shown in those cases. We can distinct these sections/results visually somehow.

ericholscher added 3 commits November 23, 2024 15:01

Show result with hit

e03a329

Add more command goodness from the response.

41ff744

humitos reviewed Dec 3, 2024

View reviewed changes

ericholscher mentioned this pull request Dec 3, 2024

Search: allow searching by file names readthedocs/readthedocs.org#11696

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial Command Pallette addon #449

Add initial Command Pallette addon #449

ericholscher commented Nov 23, 2024 •

edited by humitos

Loading

humitos commented Nov 25, 2024

ericholscher commented Nov 25, 2024

ericholscher commented Dec 2, 2024

humitos left a comment

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

humitos Dec 3, 2024

ericholscher commented Dec 3, 2024

humitos commented Dec 4, 2024

		config.addons.filesections &&
		config.addons.filesections.pages

		// Validator for FileSections Addon
		const addons_filesections = {

Add initial Command Pallette addon #449

Are you sure you want to change the base?

Add initial Command Pallette addon #449

Conversation

ericholscher commented Nov 23, 2024 • edited by humitos Loading

Demo

humitos commented Nov 25, 2024

ericholscher commented Nov 25, 2024

ericholscher commented Dec 2, 2024

humitos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericholscher commented Dec 3, 2024

humitos commented Dec 4, 2024

ericholscher commented Nov 23, 2024 •

edited by humitos

Loading