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

Theme 8px/paging #2310

Merged
merged 6 commits into from
Mar 22, 2021
Merged

Theme 8px/paging #2310

merged 6 commits into from
Mar 22, 2021

Conversation

StathamJason
Copy link
Contributor

No description provided.

@StathamJason StathamJason requested review from zhzz and dzekh March 11, 2021 08:08
Copy link
Member

@zhzz zhzz left a comment

Choose a reason for hiding this comment

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

В целом получается то что нужно. Нашел парочку моментов.

packages/react-ui/components/Paging/Paging.styles.ts Outdated Show resolved Hide resolved
packages/react-ui/internal/themes/DefaultTheme.ts Outdated Show resolved Hide resolved
packages/react-ui/internal/themes/Theme8px.ts Outdated Show resolved Hide resolved
packages/react-ui/internal/themes/Theme8px.ts Show resolved Hide resolved
@vercel
Copy link

vercel bot commented Mar 19, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

storybook-flat – ./packages/react-ui

🔍 Inspect: https://vercel.com/reactui/storybook-flat/Fj9eSJZrKsJ8tm44tjBCVLoKC77s
✅ Preview: https://storybook-flat-git-theme-8px-pagination-reactui.vercel.app

storybook-8px – ./packages/react-ui

🔍 Inspect: https://vercel.com/reactui/storybook-8px/4hEraHLguGcZUMQYoj8k1DGR6efK
✅ Preview: https://storybook-8px-git-theme-8px-pagination-reactui.vercel.app

storybook-default – ./packages/react-ui

🔍 Inspect: https://vercel.com/reactui/storybook-default/HdXueDRi3tRZzExGYvEN8roq9kR3
✅ Preview: https://storybook-default-git-theme-8px-pagination-reactui.vercel.app

@vercel vercel bot temporarily deployed to Preview – retail-ui-default March 19, 2021 11:00 Inactive
@vercel vercel bot temporarily deployed to Preview – retail-ui-default-8px March 19, 2021 11:00 Inactive
@zhzz zhzz mentioned this pull request Mar 19, 2021
14 tasks
@vercel vercel bot temporarily deployed to Preview – retail-ui-default March 19, 2021 12:19 Inactive
@vercel vercel bot temporarily deployed to Preview – retail-ui-default-8px March 19, 2021 12:19 Inactive
@StathamJason StathamJason requested a review from zhzz March 19, 2021 12:49
Copy link
Member

@zhzz zhzz left a comment

Choose a reason for hiding this comment

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

Еще похоже, что в 8px-теме высота всего Paging должна быть 48px в сумме. Там вроде вертикальных margin у ссылок нет.

image

packages/react-ui/components/Paging/Paging.styles.ts Outdated Show resolved Hide resolved
packages/react-ui/internal/themes/Theme8px.ts Outdated Show resolved Hide resolved
packages/react-ui/internal/themes/Theme8px.ts Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – retail-ui-default March 22, 2021 09:41 Inactive
@vercel vercel bot temporarily deployed to Preview – retail-ui-default-8px March 22, 2021 09:41 Inactive
@StathamJason StathamJason requested a review from zhzz March 22, 2021 10:04
@vercel vercel bot temporarily deployed to Preview – storybook-flat March 22, 2021 11:18 Inactive
@vercel vercel bot temporarily deployed to Preview – storybook-8px March 22, 2021 11:18 Inactive
@vercel vercel bot temporarily deployed to Preview – storybook-default March 22, 2021 11:18 Inactive
@zhzz
Copy link
Member

zhzz commented Mar 22, 2021

Тоже чуть зафлексил, чтобы шрифт задать в одном месте можно было.

@zhzz zhzz changed the base branch from master to theme-8px/all March 22, 2021 12:53
@zhzz zhzz merged commit 5814aa0 into theme-8px/all Mar 22, 2021
@zhzz zhzz deleted the theme-8px/pagination branch March 22, 2021 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants