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

"Fix the hover and deleting annotation issue" #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ttbt28
Copy link
Collaborator

@ttbt28 ttbt28 commented Jul 23, 2024

In this Pull Request, I already did:

  • Fix the hover issue so that we do not need to refresh the page after pulling the models to get the hover.
  • Deleting annotations works after we turn on the highlight for unannotated variables.
  • Creating a popup which contains the version of AWE when clicking on Antimony Web Editor and then About.

@ttbt28 ttbt28 requested review from evaxliu and Edih112 July 24, 2024 19:05
Copy link
Collaborator

@evaxliu evaxliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, but I noticed one bug.
When starting the website for the first time or selecting file for the first time, I need to delete the annotation twice before it registers as deleted. Otherwise, it shows up on the hover again and it didn't actually delete. Also make sure to fix your merge conflicts before requesting a PR review.

@evaxliu evaxliu requested a review from konankisa July 24, 2024 19:27
@evaxliu evaxliu requested review from konankisa and removed request for konankisa August 7, 2024 02:51
Copy link
Collaborator

@konankisa konankisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the deletion works as intended!

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

Successfully merging this pull request may close these issues.

3 participants