Skip to content

Small quality of life improvements#171

Open
lolman420 wants to merge 4 commits intomainfrom
daniel-dev
Open

Small quality of life improvements#171
lolman420 wants to merge 4 commits intomainfrom
daniel-dev

Conversation

@lolman420
Copy link
Copy Markdown
Collaborator

Made text black in the player search input box and also added disable to "create new player" button

@mUusitalo
Copy link
Copy Markdown
Contributor

mUusitalo commented Feb 10, 2025

Toi nappihomma on kova!

Hakutoiminnosta pari kyssäriä:

Ekana, voiko tolla hakea nyt useammalla nimen osalla samaan aikaan? Esim. kun killassa on miljoona Mikaelia, saanko pelkästään itseni näkyviin kirjoittamalla "Mikael Uusitalo" hakuun?

Edit: eiku hetkonen, kyl taitaa saada, eiks vaan?

Ja toiseksi: saisko ID:llä hakemisen edelleen käyttöön? Mä ainakin haen itseni kolmimerkkisellä ID:llä nimen sijaan.

Comment thread src/server/db/players/index.ts Outdated

const conditions = queryParts.map((part) => ({
[Op.or]: [
Sequelize.where(Sequelize.fn("lower", Sequelize.col("first_name")), {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Saisko tähän vielä ton ID:n takasin? Mä oon ainaki yleensä hakenu tilini nimeomaan ID:llä enkä nimelläni.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

nyt pitäis toimii, mitä mietteit

@lolman420
Copy link
Copy Markdown
Collaborator Author

joo ei ollu tarkoitus poistaa id:tä

lisäsin sen takas ja refaktoroin koodia paremmaks

nyt tää kuitenkin hajos itellä ja näyttää mul 500 xdd

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements small UI/UX improvements to enhance user experience in the player management interface. The changes focus on visual clarity and preventing accidental duplicate submissions.

  • Added text color styling to the player search input for better visibility
  • Implemented button disable functionality to prevent duplicate player creation
  • Enhanced search functionality with multi-word query support and error handling

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/app/components/ui/PlayerSearch/PlayerSearchLink.tsx Added text-black class to search input for better text visibility
src/app/components/Player/PlayerForm/index.tsx Added button disable state to prevent duplicate submissions during player creation
src/server/db/players/index.ts Enhanced search algorithm with multi-word support and error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

};

const submitNewPlayer = async (newPlayer: NewPlayer) => {
setButtonPressed(!isButtonPressed);
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

This toggle logic is incorrect. The button should be disabled during submission and re-enabled after completion. This should be setButtonPressed(true) at the start of submission and setButtonPressed(false) after the request completes (both success and error cases).

Copilot uses AI. Check for mistakes.
Comment on lines +137 to +140
const options = await Promise.all(
queryParts.map(async (part) => {
return await Promise.all(
permutations.map(async (perm) => {
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

Nested Promise.all creates unnecessary async operations. The inner permutations.map doesn't perform any async operations, so the inner Promise.all and async/await are redundant and could impact performance.

Copilot uses AI. Check for mistakes.
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.

4 participants