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

Don't double highlight nested tables #1197

Closed
wants to merge 2 commits into from

Conversation

ryanmk54
Copy link
Contributor

The TableCel is double highlighted if there is a table in a cell. This pull request makes the highlight only apply to td's that are a direct child of tr.

Copy link

vercel bot commented Jul 23, 2024

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

Name Status Preview Comments Updated (UTC)
material-react-table ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 5:53pm
material-react-table-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 5:53pm

@ryanmk54
Copy link
Contributor Author

I'm not sure what caused "Clean PR" step above to fail. I wonder if it is unable to checkout my branch? My fork is public with no branch protection. I have no idea why it wouldn't be able to check out my branch. Any help would be greatly appreciated.

@KevinVandy
Copy link
Owner

Is there a screenshot of what the bug is? Table in a cell?

@ryanmk54
Copy link
Contributor Author

ryanmk54 commented Jul 23, 2024

image

The gray is the hover color. The dark gray happens when a nested table is inside a cell, very similar to the one I added to the storybook example.

This is what I mean by table in a cell

    Cell: ({ cell }) => (
      <table>
        <tbody>
          <tr>
            <td>{cell.getValue<string>()}</td>
          </tr>
        </tbody>
      </table>
    )

I use a table in a cell to keep the text on one line with an ellipsis.

/* https://stackoverflow.com/a/19623352/2453676 */

.ellipsis-cell {
  /* magic */
  width: 100%;
  table-layout: fixed;

  /*not really necessary, removes extra white space */
  border-collapse: collapse;
  border-spacing: 0;
  border: 0;
}

.ellipsis-cell td {
  white-space: nowrap;
  overflow: hidden;
  text-overflow: ellipsis;
}

Thank you for the fast response.

@KevinVandy
Copy link
Owner

A table within a cell just to add an ellipsis is a weird solution. You should just be able to use any old div with the proper CSS to accomplish the same thing.

@ryanmk54
Copy link
Contributor Author

Thank you for encouraging me to find an alternate solution! Here is my new solution to ellipsis overflowing text for future readers.

import './ellipsis-div.css'

const EllipsisDiv = ({ cell }) => (
  <div className="ellipsis-div__parent" title={cell.getValue()}>
    <div className="ellipsis-div__child">
      {cell.getValue()}
    </div>
  </div>
)

export default EllipsisDiv

/* ./ellipsis-div.css */
.ellipsis-div__parent {
  display: table;
  table-layout: fixed;
  width: 100%;
}

.ellipsis-div__child {
  white-space: nowrap;
  overflow: hidden;
  text-overflow: ellipsis;
}

I have another use case for a table inside a cell.
image

I have a list of items with three attributes, document number, line number, and quantity. I would like them to be aligned, so it looks more organized. The same layout could be accomplished with CSS Grid, but using a table for a list of data seems easier to work with.

@KevinVandy KevinVandy closed this Jul 28, 2024
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