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

Arc 968 view repos addons #2357

Open
wants to merge 47 commits into
base: main
Choose a base branch
from
Open

Conversation

kamaksheeAtl
Copy link
Contributor

@kamaksheeAtl kamaksheeAtl commented Aug 21, 2023

What's in this PR?
add-ons and fixes:

  1. make search-bar not case sensitive
  2. re-render repo list on search clear
  3. add debouncing to search
  4. add FINISHED option in filter
Screen.Recording.2023-08-21.at.8.36.20.pm.mov

Why
Make the feature better

Added feature flags
NA

Affected issues
NA

How has this been tested?
Jest unit test cases and locally via atlas tunnel

Whats Next?

jkay-atlas and others added 30 commits March 2, 2023 11:54
make searchbar not case sensitive
re-render repo list on search clear
add debouncing to search
add FINISHED option in filter
@kamaksheeAtl kamaksheeAtl requested a review from a team as a code owner August 21, 2023 06:21
krazziekay
krazziekay previously approved these changes Aug 21, 2023
$('#repo-search').on('input', function() {
// re-render the original list after clearing the search bar
if($(this).val().length === 0) {
loadRepos(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

\nit

Suggested change
loadRepos(1)
loadRepos(1);

@@ -1,4 +1,6 @@
$(document).ready(() => {
// to get the focus back on seach bar after comp reload
Copy link
Collaborator

Choose a reason for hiding this comment

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

\nit Typo

Suggested change
// to get the focus back on seach bar after comp reload
// to get the focus back on search bar after comp reload

@@ -54,7 +54,7 @@ export const JiraGetConnectedRepos = async (
[Op.and]: [
{
repoName: {
[Op.like]: `%${repoName}%`
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi while you are here, can you also have a look at is this replacement vulnerable for the SQL injection attack? I suspect we should use something else instead of [Op.like]: `...${}...`

let repoSearchTimeoutId;
$('#repo-search').on('input', function() {
// re-render the original list after clearing the search bar
if($(this).val().length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have this block

if($(this).val().length === 0) {
			loadRepos(1);
		}

The function will still re-render the original list after clearing the search bar after 500ms? So above block is just to make it quicker on empty state?

Base automatically changed from ARC-968-view-repos to main August 31, 2023 23:32
@bgvozdev bgvozdev dismissed krazziekay’s stale review August 31, 2023 23:32

The base branch was changed.

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

8 participants