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

VaPagination rework #2283

Merged
merged 24 commits into from Sep 13, 2022
Merged

VaPagination rework #2283

merged 24 commits into from Sep 13, 2022

Conversation

aluarius
Copy link
Contributor

@aluarius aluarius commented Aug 22, 2022 •
edited

Additionally closes: #2269

Description

  • Pagination rework according to the new VaButton implementation.
  • New visual states and examples.
  • Component moved out from VaButtonGroup.
  • Docs updates.

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

@aluarius aluarius self-assigned this Aug 22, 2022
RVitaly1978
RVitaly1978 requested changes Aug 23, 2022
View reviewed changes
Copy link
Contributor

@RVitaly1978 RVitaly1978 left a comment •
edited by aluarius

Choose a reason for hiding this comment

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

Buttons have confusing styles. For example:

![chrome-capture-2022-7-23](https://user-images.githubusercontent.com/55198465/186181812-3604e6c6-a74a-46ce-a260-0b524a6e7152.gif)

packages/ui/src/components/va-pagination/VaPagination.vue Show resolved Hide resolved
packages/ui/src/components/va-pagination/VaPagination.vue Outdated Show resolved Hide resolved
packages/ui/src/components/va-pagination/VaPagination.vue Show resolved Hide resolved
packages/ui/src/components/va-pagination/VaPagination.vue Outdated Show resolved Hide resolved
@aluarius
Copy link
Contributor Author

Buttons have confusing styles.

Damn, looks strange, will investigate.

@rustem-nasyrov
Copy link
Collaborator

rustem-nasyrov commented Aug 23, 2022 •
edited by aluarius

Current page number is barely visible:

![image](https://user-images.githubusercontent.com/29167241/186191465-3abae922-9816-4cc8-a3b6-3639a4a1362c.png) ![image](https://user-images.githubusercontent.com/29167241/186192473-0b18d7df-3111-48ff-bb7c-44ee697415e5.png)

@aluarius
Copy link
Contributor Author

Current page number is barely visible

Will move VaButton out of VaButtonGroup can't set separate button state without it. Moreover, no benefits, only problems.

@aluarius aluarius marked this pull request as draft August 23, 2022 15:44
@aluarius
refactor:component moved out from va-button-group
2749219 
fix:review bugs fixes
refactor:docs updates
@aluarius aluarius marked this pull request as ready for review August 24, 2022 13:07
@m0ksem m0ksem added this to the 1.5.0 milestone Aug 27, 2022
@m0ksem m0ksem added the component Is a new component or part of existing one label Aug 27, 2022
m0ksem
m0ksem requested changes Sep 9, 2022
View reviewed changes
Copy link
Contributor

@m0ksem m0ksem left a comment •
edited by aluarius

Choose a reason for hiding this comment

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

![image](https://user-images.githubusercontent.com/23530004/189381887-3cb5688a-de3e-4da0-98b4-6f3b3c5871ef.png)

Buttons must be disabled.

Stange input

![image](https://user-images.githubusercontent.com/23530004/189382298-51fc8cba-e4e4-4c84-8290-982418c9eadd.png)

packages/ui/src/components/va-pagination/VaPagination.vue Outdated Show resolved Hide resolved
@m0ksem m0ksem mentioned this pull request Sep 12, 2022
Merged
4 tasks
asvae
asvae approved these changes Sep 13, 2022
View reviewed changes
Copy link
Contributor

@asvae asvae left a comment

Choose a reason for hiding this comment

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

Looks good!

@aluarius
refactor: review suggestions
4b6f4ea 
feat: docs updates
fix: docs translation fix
@aluarius aluarius merged commit 62adea4 into epicmaxco:release/1.5.0 Sep 13, 2022
@aluarius aluarius mentioned this pull request Sep 13, 2022
Closed
aluarius added a commit to aluarius/vuestic-ui that referenced this pull request Sep 14, 2022
@aluarius
fix: proper mixin name in VaPagination after merging epicmaxco#2394 & e…
3608ec8 
…picmaxco#2283 to the 1.5.0 branch
@aluarius aluarius mentioned this pull request Sep 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asvae asvae asvae approved these changes

@rustem-nasyrov rustem-nasyrov rustem-nasyrov approved these changes

@Derranion Derranion Awaiting requested review from Derranion

@RVitaly1978 RVitaly1978 Awaiting requested review from RVitaly1978

@m0ksem m0ksem Awaiting requested review from m0ksem

Assignees

@aluarius aluarius

Labels
component Is a new component or part of existing one
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@aluarius @rustem-nasyrov @asvae @m0ksem @RVitaly1978