lime-elements icon indicating copy to clipboard operation
lime-elements copied to clipboard
verified publisher icon Lundalogik

feat(table): introduce recent active markers

Open paulinewahle opened this issue 10 months ago • 6 comments

fix: https://github.com/Lundalogik/crm-feature/issues/1294

Summary by CodeRabbit

  • New Features
    • Recently active table rows are now visually indicated, allowing users to see not only the currently selected row but also the previous one.
  • Style
    • Enhanced table row styling with a consistent left border indicator for selected and recently active rows.

Review:

  • [ ] Commits are atomic
  • [ ] Commits have the correct type for the changes made
  • [ ] Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

  • [ ] Chrome
  • [ ] Edge
  • [ ] Firefox

Linux:

  • [ ] Chrome
  • [ ] Firefox

macOS:

  • [ ] Chrome
  • [ ] Firefox
  • [ ] Safari

Mobile:

  • [ ] Chrome on Android
  • [ ] iOS

paulinewahle avatar Apr 02 '25 13:04 paulinewahle

📝 Walkthrough

Walkthrough

The changes introduce a new SCSS mixin for a selected row indicator and a .recent-active class for table rows. In the Table component, logic is added to track and style the two most recently active rows, updating their classes to reflect their status. Minor code formatting adjustments are also made.

Changes

File(s) Change Summary
src/components/table/partial-styles/tabulator-custom-styles.scss Added table-row-selected-indicator SCSS mixin; replaced manual row indicator styling with mixin; added .recent-active class for recently active rows; imported theme color variables.
src/components/table/table.tsx Added activeRowHistory property to track most recent rows; updated row click and formatting logic to apply .recent-active class; minor formatting changes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TableComponent
    participant TabulatorRow

    User->>TableComponent: Clicks on a row
    TableComponent->>TabulatorRow: Activates clicked row
    TableComponent->>TableComponent: Push row to activeRowHistory
    TableComponent->>TableComponent: If >2, remove oldest from history
    TableComponent->>TabulatorRow: Add 'active' class to current row
    TableComponent->>TabulatorRow: Add 'recent-active' class to previous row (if any)
    TableComponent->>TabulatorRow: Remove 'recent-active' class from other rows

[!TIP]

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.

📜 Recent review details

Configuration used: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67850a00af3739022687b7dec4b00991eab4e61f and f3d65f7dc0667731442381ff39195be20d392dce.

📒 Files selected for processing (2)
  • src/components/table/partial-styles/tabulator-custom-styles.scss (3 hunks)
  • src/components/table/table.tsx (7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{tsx,scss}`: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.

**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.

  • src/components/table/partial-styles/tabulator-custom-styles.scss
  • src/components/table/table.tsx
`**/*.tsx`: Our `.tsx` files are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from the `render` method, they will sometime...

**/*.tsx: Our .tsx files are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from the render method, they will sometimes wrap them in an array literal. In these cases, rather than recommending they add key properties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special <Host> element.

  • src/components/table/table.tsx
`**/*.{ts,tsx}`: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.

**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.

  • src/components/table/table.tsx
🪛 ESLint
src/components/table/table.tsx

[error] 110-110: No magic number: 2.

(no-magic-numbers)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Docs / Publish Docs
  • GitHub Check: Build
  • GitHub Check: Test
  • GitHub Check: Lint
🔇 Additional comments (2)
src/components/table/table.tsx (2)

711-718: LGTM - Good implementation of the active row tracking

The active row history is properly maintained, keeping track of the current and previous active rows.

761-769: LGTM - Clean implementation of the recent-active class

The code properly distinguishes between the active row and the previously active row, applying the appropriate styling classes.

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Apr 02 '25 13:04 coderabbitai[bot]

The recent marker is for now only active in selectable rows and not in lists at all, for continuity the recent marker should only be where there is an active marker, which is not in lists at the moment.

paulinewahle avatar May 14 '25 09:05 paulinewahle

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3516/

github-actions[bot] avatar May 14 '25 09:05 github-actions[bot]

two things I can think of:

  1. Since we are upgrading Tabulator here https://github.com/Lundalogik/lime-elements/pull/3478 we should revisit this issue after the upgrade
  2. We should also have a more global approach here. We have other views, such as List view, Kanban view, etc… All of these should have some sort of support for indicating the "previously selected item". We can't only implement this in the table view. How would we do this best? 🤔

Kiarokh avatar Jul 10 '25 12:07 Kiarokh

two things I can think of:

  1. Since we are upgrading Tabulator here chore(table): update tabulator to 6 #3478 we should revisit this issue after the upgrade
  2. We should also have a more global approach here. We have other views, such as List view, Kanban view, etc… All of these should have some sort of support for indicating the "previously selected item". We can't only implement this in the table view. How would we do this best? 🤔

Is this a blocker to getting this PR in? Should we create issues for those other views?

john-traas avatar Jul 15 '25 11:07 john-traas

Is this a blocker to getting this PR in? Should we create issues for those other views?

I think it's just hard to remember to check and verify it in the https://github.com/Lundalogik/lime-elements/pull/3478 I'd at least wait for that PR to be merged first.

And I think we can make an umbrella issue for all views, and connect this as a sub-task to that one. We will have many different implementations in various repos, to address the issue of:

  • Hey, which item did I select before this one?
  • Oh, which item was selected, just before I deselected it?

Kiarokh avatar Jul 28 '25 12:07 Kiarokh