Skip to content

First Request#923

Open
Sany8k wants to merge 8 commits intomate-academy:masterfrom
Sany8k:master
Open

First Request#923
Sany8k wants to merge 8 commits intomate-academy:masterfrom
Sany8k:master

Conversation

@Sany8k
Copy link
Copy Markdown

@Sany8k Sany8k commented Mar 7, 2026

@Sany8k Sany8k changed the title [DEMOLINK]https://sany8k.github.io/react_phone-catalog/#/ First Request Mar 7, 2026
Copy link
Copy Markdown

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

great job, just a few fixes to make

  1. it would be better to open this link in a new tab
Image
  1. after selecting that capacity on that phone, an error page appears
Image Image
  1. add some arrows to these buttons
Image

@Sany8k Sany8k requested a review from etojeDenys March 8, 2026 11:55
Copy link
Copy Markdown

@Denys-Kravchuk9988 Denys-Kravchuk9988 left a comment

Choose a reason for hiding this comment

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

Good job!

A few things to improve:

  1. It's better to add dropdown caret for these selects
Image
  1. I would recommend to add cursor: pointer; when user hovers this section
Image

Copy link
Copy Markdown

@Denys-Kravchuk9988 Denys-Kravchuk9988 left a comment

Choose a reason for hiding this comment

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

Almost done!

One tiny bug to solve:
when I close the select's list the caret is in wrong position
Image

Copy link
Copy Markdown

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

lgtm

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.

3 participants