Skip to content

Conversation

@notAreYouScared
Copy link
Member

@notAreYouScared notAreYouScared commented Oct 4, 2025

No description provided.

@notAreYouScared notAreYouScared added this to the beta26 milestone Oct 4, 2025
@notAreYouScared notAreYouScared self-assigned this Oct 4, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 4, 2025

📝 Walkthrough

Walkthrough

Added filamentSchema.isStateChanged; Mousetrap bindings now unbind on Livewire navigation; select components gain async update version-guard and fragment rendering; tables column select uses versioning to avoid stale renders; tables initializer signature changed to add selectsGroupsOnly and renamed parameters.

Changes

Cohort / File(s) Summary
Filament Schema Utilities
public/js/filament/schemas/schemas.js
Added public method isStateChanged(e, n) that compares states via JSON.stringify inside try/catch and falls back to strict inequality on error. Minor/minified parameter renames only.
Mousetrap / App Integration
public/js/filament/filament/app.js
When Mousetrap directive is set up, binds a livewire:navigating listener that unbinds current Mousetrap bindings to ensure cleanup during Livewire navigation. Also contains minor refactors/renames.
Form Select Component (forms)
public/js/filament/forms/components/select.js
Internal identifiers renamed/refactored; no public API changes. Logic and behavior preserved.
Table Column Select (tables/components/columns)
public/js/filament/tables/components/columns/select.js
Introduced selectedDisplayVersion version guard and update serialization: updateSelectedDisplay renders into a document fragment and applies updates only if version matches to prevent stale async updates. Minor loading/no-results render adjustments.
Tables Core / Initializer
public/js/filament/tables/tables.js
Public initializer function parameter destructuring renamed and extended: added new public option selectsGroupsOnly. Internal references and entanglement wiring updated to match renamed bindings. Public signature semantics changed accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App as filament/app (Mousetrap)
  participant MT as Mousetrap
  participant LW as Livewire

  User->>App: initialize mousetrap directive
  App->>MT: bind keys (i)
  note right of App: Register listener for navigation cleanup
  LW->>App: livewire:navigating event
  rect rgba(220,200,255,0.18)
    App->>MT: unbind keys (i)  %% new cleanup path
  end
Loading
sequenceDiagram
  autonumber
  participant Component as SelectComponent
  participant API as async label fetch
  participant DOM as Document

  Component->>Component: increment selectedDisplayVersion (v)
  Component->>API: fetch labels (async)
  API-->>Component: labels (async response)
  alt response version matches v
    Component->>DOM: render into DocumentFragment
    Component->>DOM: replace target node with fragment
  else response stale
    Component-->>DOM: discard update (no DOM change)
  end
Loading

Pre-merge checks

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning No pull request description was provided, so there is no context or summary of the changes and objectives included in this PR, making it difficult for reviewers to understand what is being proposed. Please add a description that outlines the key modifications in this pull request, including the added isStateChanged method, the Mousetrap livewire:navigating handler, the internal renames, and the new selectsGroupsOnly option in the table component. Providing this context will ensure reviewers can efficiently assess the changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The provided title “v4.1.1 + upgrade” is too generic and only indicates a version bump rather than summarizing any of the functional or API changes introduced by this PR, such as the new isStateChanged utility, Mousetrap navigation cleanup, async rendering guards in selects, or the updated selectsGroupsOnly option in tables. It fails to convey the primary purpose or key enhancements of the changeset. As a result, someone scanning the repository history would not understand the main impact of this update from the title alone. Please revise the title to briefly describe the main change or feature introduced in this PR, for example highlighting the new schema state comparison method or the API signature update in the filament table component. A concise yet descriptive title will help collaborators quickly grasp the intent and scope of the update.

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@notAreYouScared notAreYouScared changed the title v4.1.1 + upgrade v4.1.2 + upgrade Oct 6, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (5)
public/js/filament/filament/app.js (1)

1-1: Unbind global Mousetrap combos too to avoid stale “global” flags after navigation

The navigating handler correctly unbinds current combos, but the Mousetrap plugin’s global map isn’t cleared, so future re-binds of the same combo will remain global unintentionally.

Add an unbindGlobal helper to the plugin and call it alongside unbind:

 (function(t){if(t){
   var n={}, i=t.prototype.stopCallback;
   t.prototype.stopCallback=function(l,g,c,_){ var y=this; return y.paused?!0:n[c]||n[_]?!1:i.call(y,l,g,c) },
   t.prototype.bindGlobal=function(l,g,c){ var _=this; if(_.bind(l,g,c), l instanceof Array){ for(var y=0;y<l.length;y++) n[l[y]]=!0; return } n[l]=!0 },
+  t.prototype.unbindGlobal=function(l){
+    if (l instanceof Array) { for (var y=0; y<l.length; y++) delete n[l[y]]; return }
+    delete n[l]
+  },
   t.init()
 }})(typeof Mousetrap<"u"?Mousetrap:void 0);

Then, in the directive’s navigating handler:

- document.addEventListener("livewire:navigating",()=>{q.default.unbind(i)},{once:!0})
+ document.addEventListener("livewire:navigating",()=>{ q.default.unbind(i); q.default.unbindGlobal?.(i) },{once:!0})
public/js/filament/schemas/schemas.js (1)

1-1: Add cheap reference-equality fast path to isStateChanged

Avoid stringify when values are already strictly equal.

- isStateChanged(e,n){if(e===void 0)return!1;try{return JSON.stringify(e)!==JSON.stringify(n)}catch{return e!==n}}
+ isStateChanged(e,n){
+   if (e === void 0) return !1
+   if (e === n) return !1
+   try { return JSON.stringify(e) !== JSON.stringify(n) }
+   catch { return e !== n }
+ }
public/js/filament/tables/components/columns/select.js (1)

1-1: Guard async search results against stale updates

Rapid typing can render out‑of‑order results. Add a request/version counter similar to selectedDisplayVersion:

- this.searchTimeout=setTimeout(async()=>{
+ const reqId = (this.searchRequestId = (this.searchRequestId ?? 0) + 1)
+ this.searchTimeout=setTimeout(async()=>{
     this.searchTimeout=null
     this.isSearching=!0
     try{
       this.showLoadingState(!0)
       let i=await this.getSearchResultsUsing(e),
           n=Array.isArray(i)?i:i&&Array.isArray(i.options)?i.options:[]
+      if (reqId !== this.searchRequestId) return
       this.options=n
       this.populateLabelRepositoryFromOptions(n)
       this.hideLoadingState()
       this.renderOptions()
       this.isOpen&&this.positionDropdown()
       this.options.length===0&&this.showNoResultsMessage()
     }catch(i){ ... }finally{ this.isSearching=!1 }
   },this.searchDebounce)
public/js/filament/tables/tables.js (2)

1-1: Align property naming: lastCheckedRecord vs lastChecked

State defines lastCheckedRecord but handlers use lastChecked. Use one consistently to avoid confusion and accidental shadowing:

- lastCheckedRecord:null,
+ lastChecked:null,
 ...
- if(!this.lastChecked){ this.lastChecked=t; return }
+ if(!this.lastChecked){ this.lastChecked=t; return }
 ...
- if(!o.includes(this.lastChecked)){ this.lastChecked=t; return }
- let u=o.indexOf(this.lastChecked), m=o.indexOf(t)
+ if(!o.includes(this.lastChecked)){ this.lastChecked=t; return }
+ let u=o.indexOf(this.lastChecked), m=o.indexOf(t)

Alternatively, rename usages to lastCheckedRecord.


1-1: New selectsGroupsOnly option is unused

The initializer accepts selectsGroupsOnly (d) but it isn’t referenced. If intentional for future use, consider removing from the public signature for now; otherwise, wire it into group selection behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 568e35b and 1ff8541.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • public/js/filament/filament/app.js (1 hunks)
  • public/js/filament/forms/components/select.js (1 hunks)
  • public/js/filament/schemas/schemas.js (1 hunks)
  • public/js/filament/tables/components/columns/select.js (1 hunks)
  • public/js/filament/tables/tables.js (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • public/js/filament/forms/components/select.js

@notAreYouScared notAreYouScared merged commit 69b669e into main Oct 6, 2025
25 checks passed
@notAreYouScared notAreYouScared deleted the charles/v4.1.1 branch October 6, 2025 10:20
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants