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

NAS-128628 / 24.10 / Add search elements for Application #10021

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

Conversation

bvasilenko
Copy link
Contributor

Testing

See ticket

@bvasilenko bvasilenko marked this pull request as ready for review May 2, 2024 20:12
@bvasilenko bvasilenko requested a review from a team as a code owner May 2, 2024 20:12
@bvasilenko bvasilenko requested review from AlexKarpov98 and removed request for a team May 2, 2024 20:12
@bugclerk bugclerk changed the title NAS-128628: Add search elements for Application NAS-128628 / 24.10 / Add search elements for Application May 2, 2024
@bugclerk
Copy link
Contributor

bugclerk commented May 2, 2024

Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 74.00%. Comparing base (c4eec1d) to head (f595969).

Files Patch % Lines
...ponents/installed-apps/installed-apps.component.ts 0.00% 13 Missing ⚠️
...-search-results/global-search-results.component.ts 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10021      +/-   ##
==========================================
- Coverage   74.01%   74.00%   -0.01%     
==========================================
  Files        1549     1550       +1     
  Lines       54048    54059      +11     
  Branches     6475     6476       +1     
==========================================
+ Hits        40002    40007       +5     
- Misses      14046    14052       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

Looks good.
Only missing a few search items:

I can't search for "Custom App" right now
Screenshot 2024-05-03 at 11 57 22

I can't search for "Add Catalog" right now
Screenshot 2024-05-03 at 11 55 49

@bvasilenko bvasilenko enabled auto-merge (squash) May 16, 2024 17:49
Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

@bvasilenko - you didn't fix what I mentioned earlier, you just pulled from master?

@AlexKarpov98
Copy link
Contributor

As well there is an issue with apps screen:
Seems like it's redenders and hides searched elements because of it.

Screen.Recording.2024-05-17.at.08.45.30.mov

Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

It works now, thanks 👍

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