Skip to content

Remote queries view using mock data #1023

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

Merged
merged 6 commits into from
Dec 1, 2021

Conversation

charisk
Copy link
Contributor

@charisk charisk commented Nov 29, 2021

This changeset builds up an initial remote queries view based on some mock data. The view will gradually be refined and hooked into real data with further PRs.

See internal issue for more context.

image

image

Checklist

N/A: This is only relevant for remote queries, which is still an internal-only feature.

  • 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.
  • @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

@charisk charisk requested a review from a team as a code owner November 29, 2021 12:05
Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Hooray, thanks for doing this! Looking much nicer already 🖼️ 😄

image

I've left some minor comments so far, but it would be good to get another review too!

Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

Nice work.

Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

This looks awesome!

@sj
Copy link
Contributor

sj commented Dec 1, 2021

Looks smashing! Can I suggest a tiny change of wording to "Summary: 13 repositories affected". It's quite common for users to write queries that are meant to explore a code base. For such queries, the term "affected" might sound a bit odd. And the number "13" is already mentioned in the text just above it.

So how about rewording "Summary: 13 repositories affected" to "Repositories with results (13)" or something like that? @nuthinking and @jf205 might have better ideas!

@charisk
Copy link
Contributor Author

charisk commented Dec 1, 2021

Looks smashing! Can I suggest a tiny change of wording to "Summary: 13 repositories affected". It's quite common for users to write queries that are meant to explore a code base. For such queries, the term "affected" might sound a bit odd. And the number "13" is already mentioned in the text just above it.

So how about rewording "Summary: 13 repositories affected" to "Repositories with results (13)" or something like that? @nuthinking and @jf205 might have better ideas!

Thanks @sj, I've gone ahead and made the change but we can also make further tweaks later on.

@charisk charisk merged commit c2316b2 into remote-queries-webview Dec 1, 2021
@charisk charisk deleted the webview-mock-data branch December 1, 2021 13:08
@nuthinking
Copy link

nuthinking commented Dec 1, 2021

I know that we already show the number above in small but, also for consistency with the results header, I'd rather go for 13 repositories with results if affected is not generic enough.

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.

5 participants