Conversation
📝 WalkthroughWalkthroughAdds server-mode datatable support: a new Changes
Sequence DiagramssequenceDiagram
actor User
participant UI as UI Component
participant State as TableState
participant Server as Server
participant Renderer as Table Renderer
User->>UI: change filter/sort/page or toggle Server Mode
UI->>State: apply TableUpdate(msg)
State->>State: check needsServerFetch(msg)
alt fetch required
State->>State: setLoading()
State-->>UI: state (loading=Loading)
UI->>Renderer: render(state)
Renderer-->>User: show spinner overlay
UI->>Server: fetchFromServer(query params)
Server-->>UI: respond(data, totalCount) or error
alt success
UI->>State: setServerData(data, totalCount)
State-->>UI: state (data, totalOverride, loading=Idle)
UI->>Renderer: render(state)
Renderer-->>User: display table
else error
UI->>State: setError(message)
State-->>UI: state (loading=Failed(message))
UI->>Renderer: render(state)
Renderer-->>User: show error alert
end
else no fetch
State-->>UI: updated client-side state
UI->>Renderer: render(state)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@forms4s-examples/src/main/scala/forms4s/example/components/DatatablePlayground.scala`:
- Around line 75-112: Guard against stale server responses by ignoring
ServerDataReceived and ServerError when the current model's dataMode !=
DataMode.Server and by adding a request token to detect out-of-order replies:
add a requestId field to the model (e.g., in the same case class that holds
tableState and dataMode), have fetchFromServer attach the current requestId to
the outgoing request and include it in the
DatatableMsg.ServerDataReceived/ServerError messages, then in the update
handlers for DatatableMsg.ServerDataReceived and DatatableMsg.ServerError check
that model.dataMode == DataMode.Server and that the response requestId matches
model.requestId before mutating tableState (otherwise ignore the message); also
increment/replace requestId whenever you call fetchFromServer (e.g., in
DataMode.Server switch and when needsServerFetch(msg) triggers).
In
`@forms4s-tyrian/src/main/scala/forms4s/tyrian/datatable/BootstrapTableRenderer.scala`:
- Around line 14-21: The dismiss button in BootstrapTableRenderer's errorAlert
uses data-bs-dismiss which requires Bootstrap JS; instead wire an onClick that
dispatches the existing TableUpdate action to clear the error (handle the
LoadingState.Failed case by replacing the button(...) with one that calls the
view's event dispatcher or message emitter to send the TableUpdate/ClearError
message), ensuring the handler updates state.loading to no-error so the alert is
removed; reference the errorAlert val, LoadingState.Failed, and the
TableUpdate/clear-error message used elsewhere in the renderer to keep behavior
consistent with other interactions.
In
`@forms4s-tyrian/src/main/scala/forms4s/tyrian/datatable/BulmaTableRenderer.scala`:
- Around line 14-21: The delete button in BulmaTableRenderer's errorNotification
(when state.loading is LoadingState.Failed) has no onClick handler, so it never
clears the error; add an onClick that dispatches a TableUpdate to clear the
loading/error state (e.g., introduce a new TableUpdate.ClearError variant and
emit it from the button's onClick), and update the table update/handler logic to
handle TableUpdate.ClearError by setting state.loading to the neutral/success
state (or None) so the notification is removed; alternatively remove the delete
button if you prefer not to support dismissal.
🧹 Nitpick comments (2)
website/docs/datatables/index.mdx (1)
81-125: Tighten the server-mode snippet to avoid an unused variable.
queryParamsis declared but not used; consider removing it or using it in the request.♻️ Suggested edit
def fetchFromServer(state: TableState[Employee]): Cmd[IO, Msg] = { - val queryParams = state.toQueryParams - // Send queryParams to your API and return DataLoaded or DataFailed + // Send query params to your API and return DataLoaded or DataFailed Cmd.Run( httpClient.get(s"/api/employees?${state.toQueryString}") .map(response => Msg.DataLoaded(response.data, response.totalCount)) .handleError(e => Msg.DataFailed(e.getMessage)) ) }forms4s-tyrian/src/main/scala/forms4s/tyrian/datatable/RawTableRenderer.scala (1)
155-167: Inconsistentstartcalculation when no data is present.Unlike
BulmaTableRendererandBootstrapTableRenderer(which both useif (state.totalFilteredItems == 0) 0 else state.page.offset + 1), this implementation will display "Showing 1 to 0 of 0 entries" when there's no data.Suggested fix for consistency
override def renderInfo[T](state: TableState[T]): Html[TableUpdate] = { - val start = state.page.offset + 1 + val start = if (state.totalFilteredItems == 0) 0 else state.page.offset + 1 val end = math.min(state.page.offset + state.page.pageSize, state.totalFilteredItems)
forms4s-examples/src/main/scala/forms4s/example/components/DatatablePlayground.scala
Show resolved
Hide resolved
forms4s-tyrian/src/main/scala/forms4s/tyrian/datatable/BootstrapTableRenderer.scala
Show resolved
Hide resolved
forms4s-tyrian/src/main/scala/forms4s/tyrian/datatable/BulmaTableRenderer.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@forms4s-examples/src/main/scala/forms4s/example/docs/DatatableTyrianExample.scala`:
- Around line 43-59: Define the missing Model type and needsServerFetch helper:
add a case class Model(tableState: TableState[Employee]) (or appropriate alias)
so update can reference model.tableState, and implement needsServerFetch(msg:
TableUpdate | whatever payload type Msg.TableMsg carries): Boolean that returns
true for actions that require server fetch (e.g., filter changes, sort changes,
page changes). Ensure the signature of needsServerFetch matches how it’s called
in update (takes the same table message type used by tableState.update), and
keep update logic unchanged so setLoading, fetchFromServer, setServerData, and
setError on TableState continue to compile.
- Around line 61-68: The fetchFromServer function references an undefined
httpClient and the file lacks proper structural closure; fix by either (a)
adding a simple placeholder HTTP client or import used by fetchFromServer (e.g.,
ensure an httpClient with a get method returning an effect compatible with
Cmd.Run is in scope) and keeping the function inside a containing object/trait
so the file compiles, or (b) if this is illustrative, move fetchFromServer (and
related types TableState[Employee] and Msg) into documentation (e.g., a .md) or
mark the file to be excluded from compilation; specifically update the code
around fetchFromServer, TableState, Employee and Msg to live inside a top-level
object (or provide the missing httpClient instance) and close the file
structure.
- Around line 38-41: The enum declaration for Msg is using invalid indentation
syntax causing "indented definitions expected" — update the enum Msg definition
to valid Scala syntax by either using brace-based form or correct indent-based
block, e.g., place the cases TableMsg, DataLoaded, and DataFailed inside a
proper enum body; ensure the case with payloads is declared as "case
TableMsg(msg: TableUpdate)" (and DataLoaded(data: Vector[Employee], totalCount:
Int), DataFailed(error: String)) so references like Msg.TableMsg resolve and the
compiler no longer errors on the enum block.
Summary by CodeRabbit
New Features
UI
Examples & Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.