-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Dataviews: Fix alignment issue of "Title" column header #68840
Dataviews: Fix alignment issue of "Title" column header #68840
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Thanks for the PR! My intuition is that the root problem is nested elements: <tr class="dataviews-view-table__row">
<th scope="col">
<span class="dataviews-view-table-header">
<button class="dataviews-view-table-header-button">Title</button>
</span>
</th>
</tr> Clickable headers should be rendered like this: <tr class="dataviews-view-table__row">
<th scope="col">
<button class="dataviews-view-table-header-button">Title</button>
</th>
</tr> It would be a good idea to investigate why you have nested elements. |
I guess we should remove the On further debugging, found that this commit - 6b16c73#diff-3021810c0ffdc124de3220e52b952b9aa2f6e604e01383e0e4c8b0aecd5e48cf, have added the I guess if we remove this span, it would not have any regression issues and our main issue would also get resolved on Also, instead of removing the CSS class (it might be used other places as well) would remove the style from that place would not be a good choice at first. Thank You, |
c2818c5
to
52a3c28
Compare
Hey @t-hamano , I've updated the PR to incorporate the suggested changes. When you have a moment, please review the latest updates. Additionally, I wanted to get your thoughts on two points:
gutenberg/packages/dataviews/src/dataviews-layouts/table/style.scss Lines 40 to 43 in 52a3c28
After searching the dataviews package, I found that the gutenberg/packages/dataviews/src/dataviews-layouts/table/style.scss Lines 159 to 161 in 52a3c28
gutenberg/packages/dataviews/src/dataviews-layouts/table/index.tsx Lines 143 to 148 in 52a3c28
Let me know what you think! Thanks for your time. |
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.
LGTM! I verified that no visual changes occurred other than to the title field.
I believe we can omit this CSS class since it was specifically targeting the element we removed.
Another thing I wanted to point out is that this inline style could be moved to the SCSS file. This would eliminate the warning, though I understand it’s outside the scope of this PR.
I think both of these are correct 👍
) * fix: Alignment issue of first header in dataviews * fix: Remove span parent of title field * fix: Remove inline css and remove extra css code Co-authored-by: im3dabasia <im3dabasia1@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org> Co-authored-by: ecgan <ecgan@git.wordpress.org>
Closes: #68838
What?
This PR addresses the issue where the "Title" column header is not properly aligned in
Dataviews
when a titleField is set. The column header shifts to the left, misaligned with the search box above and the rows below.Why?
The issue is caused by an extra margin-left on the
dataviews-view-table-header
.How?
Removes the conflicting margin-left to restore proper header alignment.
Testing Instructions
Screenshots