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

[atable] [aform] change cell style on value update #187

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

Alchez
Copy link
Collaborator

@Alchez Alchez commented Nov 6, 2024

Closes #186.


@agritheory The feature described in the ticket kinda already existed, but it wasn't reactive so a cell change wouldn't actually trigger the styles to change. This PR simply makes the styles reactive now.

I'm not sure about the intended colorization API here though, and I see a couple of options (doesn't have to be either-or):

  • Keep the status quo, and have the target application (fab, etc.) define the necessary custom CSS property (--cell-modified-color) in it's CSS stylesheets (or Vue style blocks, scoped or otherwise) at the :root level.
    • It seems like the Riverence app already defines this property in its stylesheet, so maybe this change fixes a regression?
  • Add an additional key in the table config to define the cell-modified styles, which would allow individual tables (even if there are multiple in the same Vue file) to define it's own colors.

Here's some other things to consider that may or may not be out of scope of this PR:

  • Currently, only the cell's background color is reactive. Should we consider other style properties like text color?
  • Should the applied styles be global for all cells, or should each column define it's own cell-modified schema? Maybe it accepts a function that decides the styles based on the value?

@Alchez Alchez requested a review from agritheory November 6, 2024 07:59
Copy link
Contributor

github-actions bot commented Nov 6, 2024

Coverage Report for ./atable

Status Category Percentage Covered / Total
🔴 Lines 67.5% (🎯 70%) 135 / 200
🔴 Statements 68.24% (🎯 70%) 144 / 211
🟢 Functions 76.19% (🎯 70%) 32 / 42
🔴 Branches 46.91% (🎯 70%) 76 / 162
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
atable/src/components/ACell.vue 93.54% 62.22% 100% 93.54% 101, 120, 142-145, 190
atable/src/components/index.ts 66.66% 50% 66.66% 67.44% 46-47, 60, 83-84, 106-117
Unchanged Files
atable/src/utils.ts 100% 100% 100% 100%
atable/src/components/AExpansionRow.vue 0% 0% 0% 0% 33-3
atable/src/components/ARow.vue 66.66% 56% 71.42% 63.63% 56, 63, 67, 71-85, 17
atable/src/components/ATable.vue 51.92% 32.5% 88.88% 51.02% 107-111, 117-145, 203-204
atable/src/components/ATableHeader.vue 100% 50% 100% 100%
atable/src/components/ATableModal.vue 50% 100% 0% 50% 29
Generated in workflow #409 for commit 3bcf5bb by the Vitest Coverage Report Action

@Alchez Alchez force-pushed the fix-cell-change-color branch 2 times, most recently from 689a8d2 to 7dcebb3 Compare November 6, 2024 08:48
@Alchez Alchez force-pushed the fix-cell-change-color branch from 7dcebb3 to b1d1f7a Compare November 6, 2024 08:54
@Alchez Alchez marked this pull request as draft November 6, 2024 09:43
@Alchez Alchez changed the title [wip] [atable] [aform] change cell style on value update [atable] [aform] change cell style on value update Nov 6, 2024
Copy link
Owner

@agritheory agritheory left a comment

Choose a reason for hiding this comment

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

@Alchez This should be implemented in one of the examples

@Alchez
Copy link
Collaborator Author

Alchez commented Nov 7, 2024

@agritheory I've added the --cell-modified-color property to the default ATable stories. Here's how it works:

Screencast.From.2024-11-07.12-50-05.mp4

@agritheory agritheory marked this pull request as ready for review November 7, 2024 12:25
@agritheory agritheory merged commit 8dea63d into development Nov 7, 2024
4 of 5 checks passed
@Alchez Alchez deleted the fix-cell-change-color branch November 19, 2024 09:43
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.

[ATable][AForm] Add Table level configuration for colorization on cell-level value change
2 participants