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 annotation state not being updated when annotations are saved #7

Merged
merged 1 commit into from
Jul 1, 2016

Conversation

robertknight
Copy link
Member

Annotations in the app's local state store were not being updated
after a save was successfully committed on the server.

Fixes hypothesis/h#2965

Annotations in the app's local state store were not being updated
after a save was successfully committed on the server.

Fixes hypothesis/h#2965
@robertknight
Copy link
Member Author

@seanh
Copy link
Contributor

seanh commented Jul 1, 2016

Noticed two problems:

  1. If I do a search and an annotation matches the search, and then without clearing the search I edit the annotation so that it no longer matches the search, the annotation disappears. Things vanishing when you hit save seems bad. I'd say any annotation that matched the search at the time when the search was first made should remain in the search results, until the search query is changed, even if edits to the annotation mean that it no longer matches the search. I'm not sure whether the current behaviour is as it used to be on master though?
  2. If a search has no matches an empty string is rendered: No results for “”. This is also on master though, will open a separate issue.

untitled screencast - edited 23

@seanh
Copy link
Contributor

seanh commented Jul 1, 2016

Alright, I think the two above are pre-existing issues, this pr fixes another issue where the annotation contents would revert to the previous contents on clearing a search etc. Merging this

@seanh seanh merged commit 80fb838 into master Jul 1, 2016
@seanh seanh deleted the replace-annot-when-updated branch July 1, 2016 13:49
@seanh
Copy link
Contributor

seanh commented Jul 1, 2016

Opened issues for the two pre-existing problems found above: #9 and #10

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.

None yet

2 participants