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

Show popovers for each cooking term #12

Open
Abhiek187 opened this issue Jun 29, 2022 · 1 comment
Open

Show popovers for each cooking term #12

Abhiek187 opened this issue Jun 29, 2022 · 1 comment
Labels
enhancement New feature or request
Projects

Comments

@Abhiek187
Copy link
Owner

Abhiek187 commented Jun 29, 2022

Similar to How to Stock, terms will be saved to localStorage/UserDefaults/DataStore (can move to IndexedDB/Core Data/Room with recent recipes if needed) and expire after 1 week to prevent data from being stale.

Steps to be done:

  1. On startup, check if terms are stored locally.
    a. If they are, check if they're expired.
    b. If they are, remove the terms.
  2. If no terms are stored or if they were deleted, get all terms via the API.
  3. Store the terms locally and set an expiration of 1 week.
  4. Upon opening a recipe, highlight all terms that appear on the screen and create a popover-like tool to view more information on hover/click/tap.
@Abhiek187 Abhiek187 created this issue from a note in EZ Recipes (To do) Jun 29, 2022
@Abhiek187 Abhiek187 added the enhancement New feature or request label Jun 29, 2022
@Abhiek187 Abhiek187 moved this from To do to In progress in EZ Recipes May 4, 2024
@Abhiek187
Copy link
Owner Author

Abhiek187 commented May 5, 2024

Heads up, this is a long one.

As I was working on this, I realized how complicated step 4 is. In How to Stock, I was able to add popovers to predefined spots on each page. But for this app, I need to inject popovers dynamically based on the recipe information we get. In other words, we need to:

  1. Call GET recipe.
  2. Populate the page with the recipe response.
  3. Search the text for all the terms.
  4. Inject popovers where those terms are located.

To inject the popovers, I need to add a style to make the terms look hoverable and add matTooltip to show the tooltip on hover. This involves modifying the existing HTML of the page.

There were 2 ways I thought of implementing this. One was to search the entire innerHTML of the recipe component. For each match, I would inject the style and tooltip attributes around each word and modify the innerHTML. The code looked like this:

addTooltips(recipeContent: HTMLDivElement) {
    // Look for each term throughout the recipe page and convert them into tooltips
    const terms = this.termsService.getCachedTerms();
    if (this.recipe === null || terms === null) return;
    let newInnerHTML = recipeContent.innerHTML;
    console.log('Original HTML:', newInnerHTML);

    for (const term of terms) {
      console.log(`Searching for ${term.word}...`);
      // Capture all occurrences of each term, case-insensitive
      const regex = new RegExp(`\\b(${term.word})\\b`, 'gi');
      newInnerHTML = newInnerHTML.replace(
        regex,
        `<span class="term" matTooltip="${term.definition}">$1</span>`
      );
    }

    console.log('New HTML:', newInnerHTML);
    this.renderer.setProperty(recipeContent, 'innerHTML', newInnerHTML);
  }

However, there were several issues with this approach. First, this had the potential to inject popovers outside the visible text content. For example, if one of the ingredients had an image called stock-pot.jpg, the function would pick up the word "stock" and inject a popover there. But this is just the HTML to render an image from its source. This would cause the page to look off since we modified the internal HTML (aka the image wouldn't load properly). I can't search just the textContent since I would need to know where in the HTML the text is located to inject properly. Second, the popovers wouldn't appear anyway. Since Angular encapsulates CSS classes per component, the styles for the "term" class wouldn't appear unless I add a random "ngcontent-..." attribute to the span tag. I could turn off encapsulation to make the "term" class global, but this wouldn't solve the other issue where the matTooltip wouldn't load properly. This is a special input property Angular uses to load the tooltip, and wouldn't translate directly to HTML.

The other option I came up with was to look at one word at a time and use ngIf to determine if I should add the tooltip around the word. Otherwise, I would show the text normally. This way I wouldn't need to modify the innerHTML (which is not recommended anyway since that opens the door to XSS attacks) and can rely on Angular's built-in rendering mechanism. The downside is that I would need to add this logic throughout the entire HTML, which would get tedious. So I figured I would only need to add this logic to the recipe instructions and the ingredients list. I would've included the summary as well, but that's already in HTML, so I would run into the first issue again. Below is the end result:

tooltip from hovering over simmer

I'm glad this works, but I'm worried about how this will translate to the mobile apps. I know at least for SwiftUI that there is a limit to the number of children a view can have, so I can't just add a bunch of Text views for each word. I'm also not sure how hard it will be to implement the tooltip functionality on iOS and Android. Tooltips are inherently more optimal for desktops since you hover over them. Although you could add a tap gesture to show a tooltip, this would need to be a custom behavior since it's not common on mobile (as far as I know). If push comes to shove, I might make the tooltips exclusive to the web app (but can still be interacted with on a mobile browser).

To compensate, I can make a separate page to show all the terms like a dictionary. That should be very simple to implement and I have space to add a tab to the app. Plus, I can continue to cache the terms as described above and implement the terms API on mobile so there's no waste in effort. I also feel like fetching the terms API on startup will help with improving the cold start times during tests so the server can be active before fetching the recipes.

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
Status: In progress
EZ Recipes
In progress
Development

No branches or pull requests

1 participant