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

BodyRows merge rowClassName properly #5983 #5995

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

Et7f3
Copy link
Contributor

@Et7f3 Et7f3 commented Feb 17, 2024

Fix #5983

Thanks for your quick reply and code indication @melloware,

Even though it works! I digged a bit and found the issue to happened elsewhere (the mergeProps function).

What is done in this PR:

  • add the classNameMergeFunction props because context.classNameMergeFunction was undefined (and from docs it seems to be a left over of an old doc where it wasn't nested)
  • re-order class by customisation: custom last so tailwind-merge works. I could keep the old line instead of classNames(cx('bodyRow', { rowProps: props }), rowClassName) but I think it could allow to override bg-row-odd but I am not sure.

This PR doesn't have test because I don't know how to write them (I am not a front end guy). I just tested by modifying the documentation example and see if it works.

before/after:

-bg-white text-gray-600 dark:bg-gray-900 transition duration-200 focus:outline focus:outline-[0.15rem] focus:outline-blue-200 focus:outline-offset-[-0.15rem] dark:text-white/80 dark:focus:outline dark:focus:outline-[0.15rem] dark:focus:outline-blue-300 dark:focus:outline-offset-[-0.15rem]`
+         text-gray-600 dark:bg-gray-900 transition duration-200 focus:outline focus:outline-[0.15rem] focus:outline-blue-200 focus:outline-offset-[-0.15rem] dark:text-white/80 dark:focus:outline dark:focus:outline-[0.15rem] dark:focus:outline-blue-300 dark:focus:outline-offset-[-0.15rem] bg-primary

PS: you know you can copy permalink so you can directly jumps to code

className: classNames(rowClassName, cx('bodyRow', { rowProps: props })),

Copy link

vercel bot commented Feb 17, 2024

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Feb 17, 2024 11:31pm
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Feb 17, 2024 11:31pm

Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

components/lib/hooks/useMergeProps.js Outdated Show resolved Hide resolved
components/lib/datatable/BodyRow.js Outdated Show resolved Hide resolved
@melloware melloware added the Component: Tailwind Tailwind specific issue label Feb 17, 2024
Co-authored-by: Melloware <mellowaredev@gmail.com>
@Et7f3 Et7f3 requested a review from melloware February 17, 2024 23:32
@melloware melloware merged commit a0567fb into primefaces:master Feb 17, 2024
4 checks passed
@Et7f3 Et7f3 deleted the fix_merge_rowClassName branch February 18, 2024 01:03
@melloware
Copy link
Member

melloware commented Mar 21, 2024

@Et7f3 this broke normal non-passthrough styling see #6194

@melloware
Copy link
Member

Nevermind I see what broke it and it wasn't this change @Et7f3 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Tailwind Tailwind specific issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataTable: rowClassName don't take prevalence over passthrough tailwind
2 participants