Skip to content

Conversation

@zamoore
Copy link
Contributor

@zamoore zamoore commented Jun 18, 2024

📌 Summary

If merged, this PR converts the Hds::Pagination component and its children to TS

🔗 External links

Jira ticket: https://hashicorp.atlassian.net/browse/HDS-2707

@vercel
Copy link

vercel bot commented Jun 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Aug 28, 2024 4:32pm
hds-website ✅ Ready (Inspect) Visit Preview Aug 28, 2024 4:32pm

@didoo
Copy link
Contributor

didoo commented Jun 24, 2024

@zamoore can you please move this PR to draft? the reasons I am asking are 1) we tend to avoid doing the conversion of two different/distinct components in the same PR, and 2) all the Hds::Form-related conversion will be done in the same batch (probably by @alex-ju).

PS: I am not sure if you have seen this file, it’s the list of components that need to be converted to TS, with their dependencies. If one component has a dependency, we should do the dependency first

@zamoore
Copy link
Contributor Author

zamoore commented Jun 24, 2024

@zamoore can you please move this PR to draft? the reasons I am asking are 1) we tend to avoid doing the conversion of two different/distinct components in the same PR, and 2) all the Hds::Form-related conversion will be done in the same batch (probably by @alex-ju).

PS: I am not sure if you have seen this file, it’s the list of components that need to be converted to TS, with their dependencies. If one component has a dependency, we should do the dependency first

@didoo Sure thing, would we want to wait to merge the changes to Pagination until the dropdown work is done as part of the batched form conversion work then? Or are you comfortable with merging to dropdown work now in a separate PR?

@zamoore zamoore marked this pull request as draft June 24, 2024 13:04
@zamoore zamoore removed the request for review from a team June 24, 2024 13:04
@didoo
Copy link
Contributor

didoo commented Jun 24, 2024

@zamoore can you please move this PR to draft? the reasons I am asking are 1) we tend to avoid doing the conversion of two different/distinct components in the same PR, and 2) all the Hds::Form-related conversion will be done in the same batch (probably by @alex-ju).
PS: I am not sure if you have seen this file, it’s the list of components that need to be converted to TS, with their dependencies. If one component has a dependency, we should do the dependency first

@didoo Sure thing, would we want to wait to merge the changes to Pagination until the dropdown work is done as part of the batched form conversion work then? Or are you comfortable with merging to dropdown work now in a separate PR?

I would prefer to wait if you don't mind (I made the same thing here, so don't worry: #2094)

@alex-ju
Copy link
Member

alex-ju commented Jul 17, 2024

you probably saw this @zamoore, but I ported form/select/base together with the rest of the form conversions, so this will require a rebase, then we can focus on the pagination bits

Copy link
Member

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

I did a first pass and left a couple of high-level comments. We seem to have a failing test that would need investigated. Would you be able to look into that?

Copy link
Contributor

@shleewhite shleewhite left a comment

Choose a reason for hiding this comment

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

Overall, seems good! The only blocking comment is the changelog one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants