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

Story rework: VaVirtualScroller #3860

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

xiongmao86
Copy link
Contributor

Transform VaVirtualScroller to storybook.
Part of #3676.

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)

@xiongmao86
Copy link
Contributor Author

Hi, I have most props test by step in storybook. But there are two issue I don't know how to handle:

  1. track-by prop is assign to key attribute, which is handle and remove by vue, and won't show up in the DOM of browser, how can I test for it?
  2. In the story of DifferentSizesAndMargins the bottom padding and button size is different in even and odd list items, I am not sure how to write test for it, I don't know how the size are calculated, I can work it out by reading the code, but I am not sure whether that is a good way, would it be rigid and not flex?

Is there anything I have missed to do?

@xiongmao86
Copy link
Contributor Author

Is it normal for UI Tests to fail? I haven't see anything related to VaVirtualScroller in details?

@xiongmao86 xiongmao86 changed the title Story VaVirtualScroller rework Story rework: VaVirtualScroller Sep 12, 2023
@m0ksem m0ksem requested a review from asvae September 12, 2023 19:00
Copy link
Member

@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.

Hey, great work!

There is a bit or redundancy in old demos and this PR, apart from that - things are perfect.

@asvae
Copy link
Member

asvae commented Sep 13, 2023

Is it normal for UI Tests to fail? I haven't see anything related to VaVirtualScroller in details?

Generally no (tests pass on develop). But you can ignore that for this PR 😁 as they have nothing to do with your changes (likely tests are broken because you branched off a while ago).

@asvae asvae mentioned this pull request Sep 13, 2023
75 tasks
@xiongmao86
Copy link
Contributor Author

Thanks, @asvae, How about the code now, anything I have missed?

@asvae
Copy link
Member

asvae commented Sep 13, 2023

I did a couple of small changes, but overall it's perfect 🤗.

@xiongmao86
Copy link
Contributor Author

Thank you very much, @asvae.

@asvae asvae merged commit 38bd9ab into epicmaxco:develop Sep 21, 2023
@xiongmao86 xiongmao86 deleted the fix/va-virtual-scroller-rework branch September 22, 2023 03:22
@asvae asvae mentioned this pull request Oct 3, 2023
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.

2 participants