Discard overlapped elements in hint-mode

[heiwiper]

---

Description

I have added a function to verify whether an element is overlapped when calling the follow-hint command. This function is used to discard the overlapped elements when the user has set the compute-hints-in-view-port-p to t.

Fixes #2506

Discussion

The new added function element-overlapped-p is inspired by the algorithm from saka-key project. I have noticed a side effect to this algorithm which is that it automatically discards elements that aren't in the view port, this part is already implemented in the function element-in-view-port-p which makes the process redundant since both functions are called when checking for hints. After investigating the issue I found out that the javascript function elementFromPoint returns null when the element is outside the view port, which according to the algorithm acts as if the two elements are not related thus one is overlapping the other.

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

• I have pulled from master before submitting this PR
• There are no merge conflicts.
• I've added the new dependencies as:
  • ASDF dependencies,
  • Git submodules,

    cd /path/to/nyxt/checkout
    git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser

  • and Guix dependencies.
• My code follows the style guidelines for Common Lisp code. See:
  • Norvig & Pitman's Tutorial on Good Lisp Programming Style (PDF)
  • Google Common Lisp Style Guide
• I have performed a self-review of my own code.
• My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
• Documentation:
  • All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
  • I have updated the existing documentation to match my changes.
  • I have commented my code in hard-to-understand areas.
  • I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
  • I have added a migration.lisp entry for all compatibility-breaking changes.
  • (If this changes something about the features showcased on Nyxt website) I have these changes described in the new/existing article at Nyxt website or will notify one of maintainters to do so.
• Compilation and tests:
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works. (If possible.)
  • New and existing unit tests pass locally with my changes.

[heiwiper]

I am not sure if all these tasks should be checked because some of them don't seem to be appropriate to this small change. As for the style guidelines I haven't read them yet, I will do in the coming days.

Please let me know which tasks are mandatory for the pull request to be merged.

[aadcg]

@heiwiper thanks for this PR!

I am not sure if all these tasks should be checked because some of them don't seem to be appropriate to this small change. As for the style guidelines I haven't read them yet, I will do in the coming days.

Don't worry too much about it! It's your first contribution so, realistically, no one expects you to be fully aware of all of the project's protocols.

I'll take a deeper look at your changes right now.

[aadcg]

I have noticed a side effect to this algorithm which is that it automatically discards elements that aren't in the view port, this part is already implemented in the function element-in-view-port-p which makes the process redundant since both functions are called when checking for hints. After investigating the issue I found out that the javascript function elementFromPoint returns null when the element is outside the view port

This makes sense, but I'm still confused about one detail.

Make the following test. Drop nyxt/ps:element-in-view-port-p in set-hintable-attribute (since it's redundant). Then issue the command follow-hint from the very bottom of page https://github.com/atlas-engineer/nyxt. If you scroll up, you'll see that one hint has been drawn at the beginning of the README section. I can't make sense of it but, honestly, it's not very important.

I think you're right and we should only check for nyxt/ps:element-overlapped-p.

[aadcg]

@heiwiper amazing work! I've left some comments, thanks.

[heiwiper]

Make the following test. Drop nyxt/ps:element-in-view-port-p in set-hintable-attribute (since it's redundant). Then issue the command follow-hint from the very bottom of page https://github.com/atlas-engineer/nyxt. If you scroll up, you'll see that one hint has been drawn at the beginning of the README section. I can't make sense of it but, honestly, it's not very important.

You are right about this I just noticed that behaviour, but it's apparently the same before making the changes I have added. Maybe we could open a new issue for that specific behaviour, I would gladly work on it if you think it's not very complex for a new comer.

[heiwiper]

I have added the changes locally, I will test it and update the pull request a bit later today.

[heiwiper]

I would like to thank you for reviews, I have just added the changes you have mentioned and marked the reviews as resolved except one in which I added a reply. Let me know if there are any other changes related to that one unresolved review or any other part of the code.

[aartaka]

@aadcg, can you test locally? My Nyxt misbehaves atm, so I cannot test |_・)

[aadcg]

@aadcg, can you test locally? My Nyxt misbehaves atm, so I cannot test |_・)

Sure! Don't worry, I'm going to check this later today. I'm prioritizing energy-consuming tasks and leaving the easier ones for later.

[aadcg]

@heiwiper I've merged your work via commits 2575c08 and 5a9fb1d!

Thank you again for your help :)