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

Change list of menus and add hover effect of menu #189

Merged
merged 12 commits into from
Jun 2, 2022
Merged

Conversation

Y-k-Y
Copy link
Contributor

@Y-k-Y Y-k-Y commented May 11, 2022

Resolves #179

Copy link
Member

@robonetphy robonetphy left a comment

Choose a reason for hiding this comment

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

image

@Y-k-Y
As you can see in the above image.
heading's font size and padding as well as the header height are not the same as mentioned in Figma. Pls try to make pixel perfect as mentioned in Figma

src/frontend/styles/components/header.pcss Outdated Show resolved Hide resolved
@talyguryn
Copy link
Member

something is wrong with button style

image

also there are too slow animations. do we need them?

@talyguryn
Copy link
Member

ok, button size is good now. but what is wrong with the header height?

image

@talyguryn
Copy link
Member

image

@Y-k-Y
Copy link
Contributor Author

Y-k-Y commented May 24, 2022

ok, button size is good now. but what is wrong with the header height?

image

According to Figma its height is 57px and the padding is 8px 16px so i set them as Figma. Is it wrong?

@Y-k-Y Y-k-Y requested a review from robonetphy May 30, 2022 13:51
@talyguryn
Copy link
Member

need to ask design department to check it

Copy link
Member

@robonetphy robonetphy left a comment

Choose a reason for hiding this comment

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

@Y-k-Y, we can maintain height of the header by using padding and line-height CSS properties.
you can try the changes I mentioned.

src/frontend/styles/components/header.pcss Show resolved Hide resolved
src/frontend/styles/components/header.pcss Outdated Show resolved Hide resolved
src/frontend/styles/components/header.pcss Outdated Show resolved Hide resolved
@neSpecc
Copy link
Member

neSpecc commented Jun 1, 2022

@Y-k-Y I've made some changes in Figma making menu items height similar with the Add Page button height. Please, check

image

@neSpecc
Copy link
Member

neSpecc commented Jun 1, 2022

Also, the text should be Medium

src/frontend/styles/vars.pcss Outdated Show resolved Hide resolved
@@ -3,6 +3,8 @@
--color-text-second: #5d6068;
--color-line-gray: #E8E8EB;
--color-link-active: #2071cc;
--color-link-hover: #F3F6F8;
--color-button-danger: #ff5159;
Copy link
Member

Choose a reason for hiding this comment

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

looks like an irrelevant change

Copy link
Member

Choose a reason for hiding this comment

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

Only --color-button-danger is irrelevant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@Y-k-Y Y-k-Y merged commit 9ecf8f9 into main Jun 2, 2022
@Y-k-Y Y-k-Y deleted the feature/new-header branch June 2, 2022 14:58
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.

ui: update header style
4 participants