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

Dataviews: Fix alignment issue of "Title" column header #68840

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

im3dabasia
Copy link
Contributor

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

  1. Run npm run storybook.
  2. Open the Dataviews story in Storybook.
  3. Check the header alignment for the first column.
  4. Ensure the "Title" column header is now properly aligned.

Screenshots

Before After
Screenshot 2025-01-22 at 11 25 35 PM Screenshot 2025-01-22 at 11 24 12 PM

Copy link

github-actions bot commented Jan 22, 2025

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: hbhalodia <[email protected]>
Co-authored-by: ecgan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] DataViews /packages/dataviews labels Jan 25, 2025
@t-hamano
Copy link
Contributor

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.

@hbhalodia
Copy link
Contributor

hbhalodia commented Jan 27, 2025

I guess we should remove the span which is added as a parent of the clickable field. This is because, if we scroll a bit down, we have further columns' header which are added without any extra parent span.

On further debugging, found that this commit - 6b16c73#diff-3021810c0ffdc124de3220e52b952b9aa2f6e604e01383e0e4c8b0aecd5e48cf, have added the titleField column header, since it was a new change, span was added to it.

I guess if we remove this span, it would not have any regression issues and our main issue would also get resolved on titleField not being aligned.

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,

@im3dabasia im3dabasia force-pushed the fix/68838-align-title-in-dataview branch from c2818c5 to 52a3c28 Compare February 21, 2025 15:09
@im3dabasia
Copy link
Contributor Author

Hey @t-hamano ,

I've updated the PR to incorporate the suggested changes. When you have a moment, please review the latest updates.

Screenshot image
Additionally, I wanted to get your thoughts on two points:

  1. I believe we can omit this CSS class since it was specifically targeting the element we removed.

.dataviews-view-table-header-button,
.dataviews-view-table-header {
margin-left: - #{$grid-unit-10};
}

After searching the dataviews package, I found that the dataviews-view-table-header class is used for both the title and actions table headers. However, this particular CSS rule is too specific and was only affecting the deleted , so it seems reasonable to remove it. For the actions header, there is already a corresponding CSS rule applied, attached down below.

.dataviews-view-table-header {
padding-left: $grid-unit-05;
}

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

<td
className="dataviews-view-table__checkbox-column"
style={ {
width: '1%',
} }
>

Let me know what you think! Thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] DataViews /packages/dataviews [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

titleField column header is not aligned in Dataviews
3 participants