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

Return menu prev/next navigation fuctions #232

Open
mdbraber opened this issue Jan 10, 2023 · 7 comments
Open

Return menu prev/next navigation fuctions #232

mdbraber opened this issue Jan 10, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@mdbraber
Copy link
Contributor

mdbraber commented Jan 10, 2023

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
Copy link
Contributor Author

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

mgsloan added a commit that referenced this issue Jan 14, 2023
mgsloan added a commit that referenced this issue Jan 14, 2023
@mgsloan
Copy link
Owner

mgsloan commented Jan 14, 2023

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

And thanks for the support!

@mgsloan mgsloan closed this as completed Jan 14, 2023
@mgsloan mgsloan added the enhancement New feature or request label Jan 14, 2023
@llinfeng
Copy link

@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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

mdbraber commented Jan 19, 2023

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
Copy link
Owner

mgsloan commented Jan 24, 2023

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.

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

No branches or pull requests

3 participants