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.

Sign up for GitHub

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

Jump to bottom

Apply code review comments #817

Closed
NoamGaash opened this issue Jun 18, 2024 · 5 comments
Closed

Apply code review comments #817

NoamGaash opened this issue Jun 18, 2024 · 5 comments
Assignees
@YuvalMasada
Labels
CSS related to the design, without affecting business logic frontend frontend developers issue good first issue Good for newcomers low priority usability

Comments

@NoamGaash
Copy link
Member

We're about to merge this PR:
#791

There are some minor improvements that I commented about, and I'm not sure the author of the pull request wants to fix. Never the less, he made a great feature and we want it merged.

Therefore, if anyone would like to proceed and finish up this task, it would be great. See the comments here:
#791
@NoamGaash NoamGaash added good first issue Good for newcomers CSS related to the design, without affecting business logic usability frontend frontend developers issue low priority labels Jun 18, 2024
@YuvalMasada YuvalMasada self-assigned this Jun 19, 2024
@YuvalMasada
Copy link
Collaborator

I'll take it if that's ok.
I saw another issue with the size of the modal and I'll fix it too.
It needs scroll for y-axis, at least from what I've experienced.

@NoamGaash
Copy link
Member Author

@YuvalMasada thank you!

@YuvalMasada
Copy link
Collaborator

@NoamGaash
Hi,
I've made some local changes to this component.
I didn't change the functionality, but change the used material-ui component so dark mode will be handled automatically & and also it solves the need for adding scroll bar.
I'm adding some screenshot and if that good I'll make new branch for it.

80 percent zoom dark mode
80 percent zoom - light mode
100 percent - dark mode

@NoamGaash
Copy link
Member Author

@YuvalMasada looks great! Where can I see the code? 👏

@YuvalMasada
Copy link
Collaborator

@NoamGaash
Here -> #852

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@YuvalMasada YuvalMasada

Labels
CSS related to the design, without affecting business logic frontend frontend developers issue good first issue Good for newcomers low priority usability
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@YuvalMasada @NoamGaash