mgsloan / todoist-shortcuts

Browser extension which adds comprehensive keyboard shortcuts to Todoist, beyond what is offered by Todoist itself
MIT License
313 stars 23 forks source link

Return menu prev/next navigation fuctions #232

Open mdbraber opened 1 year ago

mdbraber commented 1 year ago

In v169 navigation for left menu items was removed. I was actually using those (but didn't notice as I moved away from Todoist for a while). With some minor additions this code can be brought be back. I've written the code below integrating the nextNavItem / prevNavItem code and renaming everything to refer to (left)MenuItem.

The code is mostly similar to the code before - the main 'change' is to check for > 6 classes to determine the current menu item, rather than > 1 classes. It's still an ugly hack, but works quite well. I would be very grateful if this code could still be an official part of todoist-shortcuts!

  // Cycles down through menu items.
  function nextLeftMenuItem() {
    withLeftMenuItems((menuItems, current) => {
      // If on the last item, or no item, select the first item.
      if (current >= menuItems.length - 1 || current < 0) {
        menuItems[0].click();
      // Otherwise, select the next item.
      } else {
        menuItems[current + 1].click();
      }
    });
  }

  // Cycles up through top sections (inbox / today / next 7 days + favorites).
  function prevLeftMenuItem() {
    withLeftMenuItems((menuItems, current) => {
      // If on the first item, or no item, select the last item.
      if (current <= 0) {
        menuItems[menuItems.length - 1].click();
      // Otherwise, select the previous item.
      } else {
        menuItems[current - 1].click();
      }
    });
  }

  // Run a function on the array of left menu items, along with the index of the
  // currently selected one, if any.
  function withLeftMenuItems(f) {
    withId('top-menu', (topItems) => {
      const favoritesPanel = withId('left-menu-favorites-panel', (panel) => { return panel });
      const projectsPanel = withId('left-menu-projects-panel', (panel) => { return panel });
      withLeftMenuItemLinks([topItems, favoritesPanel, projectsPanel], f);
    });
  }

  function withLeftMenuItemLinks(containers, f) {
    const links = [];
    let current = -1;
    const allCurrents = [];
    for (const container of containers) {
      withTag(container, 'li', (item) => {
        const link =
              getFirstClass(item, 'item_content') ||
              getFirstTag(item, 'a') ||
              getFirstClass(item, 'name');
        if (hidden(item)) {
        } else if (!link) {
          warn('Didn\'t find link in', item.innerHTML);
        } else {
          links.push(link);
          const firstChild = item.firstElementChild;
          // Terrible hack around obfuscated class names.
          if (matchingClass('current')(item) ||
              (firstChild.tagName === 'DIV' &&
               !firstChild.classList.contains('arrow') &&
               firstChild.classList.length >= 6)) {
              if (!allCurrents.length) {
                current = links.length - 1;
            }
            allCurrents.push(item.innerHTML);
          }
        }
      });
    }
    if (allCurrents.length > 1) {
      warn('Multiple current menu items: ', allCurrents);
    }
    f(links, current);
  }
mdbraber commented 1 year ago

Maybe it's better use 'Sidebar' instead of 'LeftMenu'

mgsloan commented 1 year ago

Thank you!! I really appreciate the code contribution, included it in version 170.

And thanks for the support!

llinfeng commented 1 year ago

@mgsloan Bug report: if I mark a project as "Favorite" and keep this project at the top of the Projects list, repeated pressing of backtick/tilde will trap me in a loop, as below: Loop_with_favorite_section_pinned

mdbraber commented 1 year ago

Hmm, this seems to be because of how Todoist behaves: it makes a project current both in the favorites list and in the projects list (which isn't too weird as it's the same project). Problem is that the next / prev shortcut doesn't know where it is and starts at the first item again, resulting in the loop.

I can see two possibilities: split the shortcut back in two parts: one for top/favorites and one for projects. Or figure out a way to keep state whether the prev/next shortcut was moving in the favorites or projects section (which might be difficult as users can also do other interactions that move current)

llinfeng commented 1 year ago

Hmm, this seems to be because of how Todoist behaves: it makes a project current both in the favorites list and in the projects list (which isn't too weird as it's the same project). Problem is that the next / prev shortcut doesn't know where it is and starts at the first item again, resulting in the loop.

Thank you for clarifying!

I can see two possibilities: split the shortcut back in two parts: one for top/favorites and one for projects. Or figure out a way to keep state whether the prev/next shortcut was moving in the favorites or projects section (which might be difficult as users can also do other interactions that move current)

If I may, I'd vote for having backtick/tilde for stepping through entries in Favorites, and using -/= or [/] for navigating adjacent Projects.

mdbraber commented 1 year ago

Took a moment to figure out what to do, but I've used a state variable lastLeftMenuItem to remember where we were. I've also created code so you can either use nextLeftMenuItem / prevLeftMenuItem with the default sections set by NAVIGATE_LEFT_MENU_SECTIONS or use nextLeftMenuItemSections(sections) / prevLeftMenuItemSections(sections) with sections being an array of those sections you want to navigate (so you can customize your own shortcuts).

I'll leave it to @mgsloan to decide if this code would be fit to integrate.


  // Default left menu sections to navigate with prevLeftMenuItem / nextLeftMenuItem
  const NAVIGATE_LEFT_MENU_SECTIONS = ['top','favorites','projects']

  // Cycles down through menu items.
  function nextLeftMenuItem() {
    nextLeftMenuItemSections(NAVIGATE_LEFT_MENU_SECTIONS);
  }

  function nextLeftMenuItemSections(sections) {
    withLeftMenuItems(sections, (menuItems, current) => {
      // If on the last item, or no item, select the first item.
      if (current >= menuItems.length - 1 || current < 0) {
        menuItems[0].click();
      // Otherwise, select the next item.
      } else {
        menuItems[current + 1].click();
      }
    });
  }

  // Cycles up through top sections (inbox / today / next 7 days + favorites).
  function prevLeftMenuItem() {
    prevLeftMenuItemSections(NAVIGATE_LEFT_MENU_SECTIONS);
  }

  function prevLeftMenuItemSections(sections) {
    withLeftMenuItems(sections, (menuItems, current) => {
      // If on the first item, or no item, select the last item.
      if (current <= 0) {
        menuItems[menuItems.length - 1].click();
      // Otherwise, select the previous item.
      } else {
        menuItems[current - 1].click();
      }
    });
  }

  // Run a function on the array of left menu items, along with the index of the
  // currently selected one, if any.
  function withLeftMenuItems(navigateSections, f) {
    let sections = [];
    if (navigateSections.includes('top')) { sections.push(getById('top-menu')); }
    if (navigateSections.includes('favorites')) { sections.push(getById('left-menu-favorites-panel')); }
    if (navigateSections.includes('projects')) { sections.push(getById('left-menu-projects-panel')); }
    withLeftMenuItemLinks(sections, f);
  }

  let lastLeftMenuItem = -1;

  function withLeftMenuItemLinks(containers, f) {
    const links = [];
    let current = -1;
    const allCurrents = [];
    for (const container of containers) {
      withTag(container, 'li', (item) => {
        const link =
              getFirstClass(item, 'item_content') ||
              getFirstTag(item, 'a') ||
              getFirstClass(item, 'name');
        if (hidden(item)) {
        } else if (!link) {
          //warn('Didn\'t find link in', item);
        } else {
          links.push(link);
          const firstChild = item.firstElementChild;
          // Terrible hack around obfuscated class names.
          if (matchingClass('current')(item) ||
              (firstChild.tagName === 'DIV' &&
               !firstChild.classList.contains('arrow') &&
               firstChild.classList.length >= 6)) {
              if (!allCurrents.length) {
                  current = links.length - 1;
              } else if (links.length - 2 == lastLeftMenuItem) {
                  current = lastLeftMenuItem +1;
              } else if (links.length == lastLeftMenuItem) {
                  current = lastLeftMenuItem - 1;
              }
            allCurrents.push(item);
          }
        }
      });
    }
    lastLeftMenuItem = current;
    f(links, current);
  }
mgsloan commented 1 year ago

Thanks for working on this @mdbraber! Feel free to open PRs in the future in general for stuff. Please do check out https://github.com/mgsloan/todoist-shortcuts/blob/master/development.md for info on running ./eslint.sh to check formatting

I haven't thought it through properly to check, but I think this will get stuck when the duplicated item is also consecutive, since it is writing down lastLeftMenuItem before shifting the index.

Another, though minor, issue is that this won't work correctly when the contents of the left menu changes.

How about only writing down the name of the last container (and update this correctly when making a new selection)? Then, when the scan encounters a duplicate for the current selection, prefer the one that matches the last container.

Feel free to work on this, the earliest I might get around to it is this weekend.