rapi-doc / RapiDoc

RapiDoc -WebComponent for OpenAPI Spec
https://rapidocweb.com
MIT License
1.66k stars 278 forks source link

Standardize toggle on click of left panel #1028

Open majoshi1 opened 2 weeks ago

majoshi1 commented 2 weeks ago

Here is the enhancement to standardize toggle behavior on click of left panel.

Params

      render-style="focus"
      on-nav-tag-click="show-description"

Current behavior On click of left panel entry, right panel shows details. But to expand the item on left panel, user needs to click on arrow icon on right. Also, that does not collapse other items. This is inconvenient for users.

Proposed behavior On click of left panel entry, do the following: -Show details on right panel. -Expand the item on left panel. -Collapse any other open item/s on left panel.

Live example with proposed behavior: https://majoshi1.github.io/rapidocweb/examples/focused-mode.html

Below are all the changes needed for this proposed behavior.

In src\templates\navbar-template.js:

From:

export function navBarClickAndEnterHandler(event) {
  if (!(event.type === 'click' || (event.type === 'keyup' && event.keyCode === 13))) {
    return;
  }
  const navEl = event.target;
  event.stopPropagation();
  if (navEl.dataset?.action === 'navigate') {
    this.scrollToEventTarget(event, false);
  } else if (navEl.dataset?.action === 'expand-all' || (navEl.dataset?.action === 'collapse-all')) {
    expandCollapseAll(event, navEl.dataset.action);
  } else if (navEl.dataset?.action === 'expand-collapse-tag') {
    expandCollapseNavBarTag(navEl, 'toggle');
  }
}

To:

export function navBarClickAndEnterHandler(event) {
  if (!(event.type === 'click' || (event.type === 'keyup' && event.keyCode === 13))) {
    return;
  }
  const navEl = event.target;
  event.stopPropagation();
  if (navEl.dataset?.action === 'navigate') {
    this.scrollToEventTarget(event, false);
    const tagAndPathEl = navEl?.closest('.nav-bar-tag-and-paths');
    let wasExpanded = false;
    if (tagAndPathEl) {
      wasExpanded = tagAndPathEl.classList.contains('expanded');
    }
    expandCollapseAll(event, 'collapse-all');
    expandCollapseNavBarTag(navEl, 'toggle');
    if (wasExpanded) {
      expandCollapseNavBarTag(navEl, 'toggle');
    }
  } else if (navEl.dataset?.action === 'expand-all' || (navEl.dataset?.action === 'collapse-all')) {
    expandCollapseAll(event, navEl.dataset.action);
  } else if (navEl.dataset?.action === 'expand-collapse-tag') {
    expandCollapseNavBarTag(navEl, 'toggle');
  }
}

From:

<div class='nav-bar-tag-icon' tabindex='0' data-action='expand-collapse-tag'></div>

To:

<div class='nav-bar-tag-icon' style="pointer-events:none;"></div>
majoshi1 commented 2 weeks ago

Raised following PR for this change. https://github.com/rapi-doc/RapiDoc/pull/1029

mrin9 commented 2 weeks ago

Thanks for the PR !!!

though I like to disagree on "Standardize" part

I think you are trying to keep always only one tag expanded and others close automatically. I have got mixed reviews about this kind of navigation behavior. For a small spec it may work fine, but for larger spec users like to expand multiple tags at the same time and to compare their paths (navbar can show paths too) or have related API from different tags open to keep them in the context while reading the docs.

Having said that here are my concerns and suggestion

majoshi1 commented 2 weeks ago

Thanks for the quick response. Below are responses to your comments.

expand multiple tags at the same time

With focused mode, the idea is that user wants to focus on one tag at a time.

x-tag-expanded vendor-extension

With focused mode, all tags will be closed initially. In fact, this should be default behavior for focused mode.

on-nav-tag-click option

This option becomes redundant, as idea is to do both expand/collapse and show description.

expanding a tag automatically closes the other tags.

The focused mode actually needs this behavior. Also, this is standard behavior for other libraries e.g. see this demo.