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

Refactor how themes are provided. Fix a bug with tab switching. #53

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

mddragnev
Copy link
Member

Closes #21

Copy link
Member

@skrustev skrustev left a comment

Choose a reason for hiding this comment

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

These are the few issues i've found:

  1. The height of the grids in each tabs is different. Event if its not 100%, at least should be consistent.

  2. The icon in the tabs download button is now not visible, except for the currently selected one:
    image

  3. The theme for all grids is not correctly rendered. The root material theme leaks onto the other grid themes:
    For example how it is for the HR Portal:
    image
    How should be(except the trial marks ofc):
    image

@mddragnev mddragnev requested a review from skrustev February 3, 2025 16:57
@sbayreva
Copy link

sbayreva commented Feb 4, 2025

  1. There should be Top, Bottom, Left and Right paddings of 16px around the container that holds all the tabs. Now there is a padding between the tabs container and the grid, but it is 24px which is not according to the design.
    Implementation:
    Screenshot 2025-02-04 at 13 22 26

Design:
Screenshot 2025-02-04 at 13 26 14

2. Тhe background color of the tabs container should be surface while (Material)

  1. In the ERP/Inventory grid, I can still see stretched images. Not all of them but some of them:

Screenshot 2025-02-04 at 13 33 54
Screenshot 2025-02-04 at 13 58 07

  1. Fleet Management. The image here does not fit right. Maybe the height should take 100%
    Screenshot 2025-02-04 at 14 02 01

@sbayreva sbayreva self-assigned this Feb 4, 2025
@mddragnev
Copy link
Member Author

@sbayreva,
For 3. you can pull latest and look at the images. They should be ok.
For others (1,2,4) could you please log them as a separate issues since they are not part of the current effort.

@IMinchev64 IMinchev64 mentioned this pull request Feb 4, 2025
@dkamburov dkamburov merged commit c4fc9b7 into vnext Feb 4, 2025
2 checks passed
@dkamburov dkamburov deleted the mdragnev/refactor-themes branch February 4, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Going from one tab to the other throws an ExpressionChangedAfterItHasBeenCheckedError
7 participants