Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

LF-4538: Breed sorting does not work in inventory table #3558

Conversation

SayakaOno
Copy link
Collaborator

Description

  • Update descendingComparator to handle undefined and null as expected.
  • Add tests. (pnpm run test sort)

Jira link: https://lite-farm.atlassian.net/browse/LF-4538

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@SayakaOno SayakaOno added the bug Something isn't working label Dec 3, 2024
@SayakaOno SayakaOno self-assigned this Dec 3, 2024
@SayakaOno SayakaOno requested review from a team as code owners December 3, 2024 00:00
@SayakaOno SayakaOno requested review from antsgar and Duncan-Brain and removed request for a team December 3, 2024 00:00
Copy link
Collaborator

@antsgar antsgar left a comment

Choose a reason for hiding this comment

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

Nice fix!! 🚀

Copy link
Collaborator

@Duncan-Brain Duncan-Brain left a comment

Choose a reason for hiding this comment

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

👏 Looks great!

Only question is about the renaming of the type, as I am still learning TS!

Since actual type is pretty simple ([key in T]: any;) and has not changed... is it better to keep the generic name?

I might expect something named "Comparable" to check if the types are comparable like : https://stackoverflow.com/questions/53807517/how-to-test-if-two-types-are-exactly-the-same

@Duncan-Brain Duncan-Brain added this pull request to the merge queue Dec 4, 2024
Merged via the queue into integration with commit fc62aaa Dec 4, 2024
5 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants