-
Notifications
You must be signed in to change notification settings - Fork 32
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
[Datahub]: Add a list layout to other links section when many links are present #873
Conversation
step dots are now centered below but still absolute positioned step dots have a larger interactive area makes it standalone adapt tests
Affected libs:
|
📷 Screenshots are here! |
This component offers simple pagination on a list of blocks
Also do not include it in the ui-inputs module; as a standalone component, it should be imported wherever this is necessary
3346bdb
to
06c88c8
Compare
06c88c8
to
1ef846f
Compare
This was hiding the underlying user feebacks form
describe('set initial height as min height, keeps value when height changes', () => { | ||
beforeEach(() => { | ||
Object.defineProperties(component.blockContainer.nativeElement, { | ||
clientHeight: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to do this because jsdom doesn't actually compute the width and height of things; everything is always 0 in tests
I reviewed the code and beside the refactoring of the multiple Good job ! I can't wait for it to be merged 😃 |
@rcaplier thanks for the review! I added a commit to use |
Closes #771 |
Description
This PR is a rework of #864.
Additional changes were:
@rcaplier let me know if these changes make sense considering your previous work. I realized that your changes regarding the layout of the carousel (letting it overflow by default) made the whole thing much simpler to use in the datahub. Also having a paginated list was not as trivial as I thought; I went with a generic BlockList component in order to not have a UI component that know too much about what is using it, but apart from that your original approach made sense.
Replaces #864
Screenshots
Quality Assurance Checklist
breaking change
labelbackport <release branch>
labelThis work is sponsored by IGN.