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

Updated sorting icon functionality in SortBy.tsx #884

Merged
merged 8 commits into from
Feb 22, 2025

Conversation

swastika1011
Copy link
Contributor

Fixed sorting icon in SortBy.tsx
Resolves #856

Add the PR description here.
Updated the sorting icon in SortBy.tsx to use the correct component.

Before:
Screenshot (347)
Screenshot (346)

After:
Screenshot (344)
Screenshot (34

Copy link
Collaborator

@aramattamara aramattamara left a comment

Choose a reason for hiding this comment

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

Thanks for this PR!
Please ensure that both icons are the same size for consistency. Also, kindly check the border radius to maintain a uniform design.
Screenshot 2025-02-18 at 2 34 10 PM
Screenshot 2025-02-18 at 2 35 53 PM

@swastika1011
Copy link
Contributor Author

Okay I'll make the necessary changes and update it.

@swastika1011
Copy link
Contributor Author

Hi @aramattamara, I have made few changes and tried to align it. If something doesn't seem right please let me know, I'll try to resolve it.
Screenshot (377)

Screenshot (376)

@aramattamara
Copy link
Collaborator

aramattamara commented Feb 21, 2025

Looks nice! Thanks! Please fix tests and can be merged.

@swastika1011
Copy link
Contributor Author

Hey @aramattamara can you please check this now

kasya
kasya previously approved these changes Feb 22, 2025
Copy link
Collaborator

@kasya kasya 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 to me! @aramattamara approve it when you think it's ready to go - I'll set it up to merge after your approval.

@kasya kasya enabled auto-merge February 22, 2025 02:31
aramattamara
aramattamara previously approved these changes Feb 22, 2025
Copy link
Collaborator

@aramattamara aramattamara left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

This is not supposed to require my approval. But it contains package-lock.json changes that are unrelated to the PR.

I'll take care of that.

@aramattamara @kasya thanks for reviewing!

@arkid15r arkid15r dismissed stale reviews from aramattamara and kasya via 968e89e February 22, 2025 02:59
@kasya kasya added this pull request to the merge queue Feb 22, 2025
Merged via the queue into OWASP:main with commit 486d8e5 Feb 22, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update icon for sorting component
4 participants