Skip to content

Conversation

araujogui
Copy link
Member

Description

Create CSS-only responsive table

Validation

image image

Related Issues

Fixes #7656

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@Copilot Copilot AI review requested due to automatic review settings August 21, 2025 02:06
@araujogui araujogui requested review from a team as code owners August 21, 2025 02:06
Copy link

vercel bot commented Aug 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
nodejs-org Error Error Aug 27, 2025 9:36pm

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements CSS-only responsive tables that adapt to different screen sizes using media queries and data attributes. The main change transforms tables from traditional grid layout on desktop to card-based layout on mobile screens.

  • Creates mobile-friendly table layouts that stack information vertically on small screens
  • Adds automatic data-label attributes to table cells via a remark plugin
  • Updates existing table component to use internationalized labels

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/ui-components/src/styles/markdown.css Implements responsive table CSS with mobile card layout and desktop grid styles
apps/site/util/table.ts Creates remark plugin to automatically add data-label attributes to table cells
apps/site/package.json Adds required dependencies for markdown processing utilities
apps/site/next.mdx.plugins.mjs Integrates the new table plugin into the MDX processing pipeline
apps/site/components/Releases/PreviousReleasesTable.tsx Updates table component to use internationalized data-labels and fix rendering issues
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@araujogui araujogui mentioned this pull request Aug 21, 2025
5 tasks
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.90%. Comparing base (a5ea254) to head (d9f70ff).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8098      +/-   ##
==========================================
+ Coverage   75.89%   75.90%   +0.01%     
==========================================
  Files         113      113              
  Lines        9459     9459              
  Branches      307      307              
==========================================
+ Hits         7179     7180       +1     
+ Misses       2279     2278       -1     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

<>
<tr key={release.major}>
<td data-label="Version">v{release.major}</td>
<Fragment key={release.major}>
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes the error Each child in a list should have a unique "key" prop.

Copy link
Member

@avivkeller avivkeller Aug 21, 2025

Choose a reason for hiding this comment

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

(Shameless plug here, disclaimer: I wrote this package, so no need to use it if you don't want)

https://www.npmjs.com/package/remark-table-cell-titles

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a good idea. Your implementation already has tests, etc., but let's see what the rest of the team thinks first

Copy link
Member

Choose a reason for hiding this comment

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

+1 for this package

@avivkeller
Copy link
Member

avivkeller commented Aug 21, 2025

image

Can you add text wrapping on the title?

@araujogui araujogui force-pushed the feat/responsive-table2 branch from 38cccee to d9f70ff Compare August 27, 2025 21:36
@araujogui
Copy link
Member Author

CC @nodejs/nodejs-website

@@ -30,4 +32,5 @@ export const REMARK_PLUGINS = [
remarkHeadings,
// Calculates the reading time of the content
readingTime,
remarkTableTitles,
Copy link
Member

Choose a reason for hiding this comment

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

nit: Add a comment above explain this, like all the other plugins do:

Suggested change
remarkTableTitles,
// Adds a `data` attribute to table cells for our custom CSS
remarkTableTitles,

@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Aug 28, 2025
* Remark plugin that adds data-label attributes to table cells (td)
* based on their corresponding table headers (th).
*/
export default function remarkTableTitles() {
Copy link
Member

Choose a reason for hiding this comment

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

Excellent approach!

Copy link
Contributor

github-actions bot commented Aug 28, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 97 🟢 100 🟢 100 🟢 100 🔗
/en/about 🟢 97 🟢 97 🟢 100 🟠 88 🔗
/en/about/previous-releases 🟢 97 🟢 93 🟢 100 🟢 100 🔗
/en/download 🟢 93 🟢 100 🟢 100 🟢 100 🔗
/en/download/archive/current 🟢 100 🟢 100 🟢 100 🟢 100 🔗
/en/blog 🟢 98 🟢 100 🟢 96 🟢 100 🔗

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM! Good stuff and great effort put in here :)

Appreciate you going back-and-forth on this. Looking back, this bec came much simpler and easier than the original PR :)

@ovflowd
Copy link
Member

ovflowd commented Aug 28, 2025

I just looked the latest commit, is it supposed to look like this?

image image

@ovflowd
Copy link
Member

ovflowd commented Aug 28, 2025

(Looks a bit weird tho)

@araujogui
Copy link
Member Author

I just looked the latest commit, is it supposed to look like this?
image image

It's a bug, i'll take a look

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

LGMT

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.

Poor Table UX on Mobile
4 participants