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 error from "Open Query Results" button #909

Merged
merged 2 commits into from
Jul 13, 2021

Conversation

shati-patel
Copy link
Contributor

@shati-patel shati-patel commented Jul 12, 2021

Fixes #906.

The "Open Query Results" button in the query history view currently doesn't work. Error: Cannot read property 'result' of undefined (codeQLQueryHistory.itemClicked).

I experimented a bit with the handleItemClicked, and it looks like changing singleItem to finalSingleItem works as expected 🙂

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

@adityasharad
Copy link
Contributor

adityasharad commented Jul 12, 2021

Good catch, this looks sensible! Maybe we should have a query for this kind of pattern (later).

@shati-patel shati-patel marked this pull request as ready for review July 12, 2021 17:24
@shati-patel shati-patel requested a review from a team as a code owner July 12, 2021 17:24
@shati-patel
Copy link
Contributor Author

shati-patel commented Jul 12, 2021

Aside: There was another point raised in the bug report:

Though to be honest, I am not sure if the "Open Query Results" button adds any value in the first place, since you would most likely first have to select the entry from the history (by left clicking?), but by doing so you already opened the query results.

I agree that the button doesn't add any extra functionality, but I assume it was added for a reason 😄 I guess it makes the command more obvious/discoverable?

@adityasharad
Copy link
Contributor

I assume it was added for a reason 😄 I guess it makes the command more obvious/discoverable?

Looking at git blame on that line I think it was added so that we could have UI icons in the query history box for some of the common commands.

@shati-patel shati-patel merged commit 977b061 into main Jul 13, 2021
@shati-patel shati-patel deleted the shati-patel/query-results branch July 13, 2021 08:04
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.

"Open Query Results" button causes error: "Cannot read property 'result' of undefined"
2 participants