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

WCMS-22095: Clean up /data route components #251

Merged
merged 2 commits into from
Sep 17, 2024
Merged

WCMS-22095: Clean up /data route components #251

merged 2 commits into from
Sep 17, 2024

Conversation

brdunfield
Copy link
Contributor

@brdunfield brdunfield commented Sep 13, 2024

Let me know if you need an alpha release.

This PR cleans up some unneeded code from the migration to the new state management. It also updates the loading states on the /data route to match the data table tab page. (While loading, a Spinner should appear in the data table, results text should not appear, and Pagination should not appear)

Copy link
Member

@dgading dgading 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, just one tiny margin request.

</Link>
)}
</div>
</div>
<div className="ds-l-row ds-u-align-items--center">
<div className="ds-l-col--12 ds-u-display--flex ds-u-justify-content--between ds-u-align-items--center">
<div className="ds-u-font-weight--bold ds-u-margin-bottom--2">
Copy link
Member

Choose a reason for hiding this comment

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

The ds-u-margin-bottom--2 should probably move up to the parent container. When the results disappear during loading so does the margin bottom making the whole section move a little.

@brdunfield brdunfield merged commit 38912ad into main Sep 17, 2024
1 check passed
@brdunfield brdunfield deleted the WCMS-22095 branch September 17, 2024 14:04
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