Dispatch change event from Pagination and Select

[theetrain]

Closes #1491

Pagination

• Feature: dispatch change event from <Pagination> whenever user interacts with "previous" button, "next" button, page dropdown, or page size dropdown

Select

• Feature: forward change event from <Select>; necessary for Pagination to react to changes
• Fix (or breaking change?): dispatch input instead of change

Side note, it would be nice if the note I added to the @event JSDoc appeared on the rendered docs. Perhaps I can open a separate issue for that.

[metonym]

Thanks for taking the time to work on this.

A couple of things:

• I don't think breaking changes need to be made to Select, since it dispatches change as is
• For the "change" event in Pagination, I think it would be nicer to disambiguate what actually fire the event. Could you provide a code sample of how you would use it?

[theetrain]

@metonym thanks for following up. I think I can address both your points with an example:

Given a page with two pagination toolbars set up with on-demand data loading
When the user changes pagination via next/prev buttons, or the page dropdown
Then the on:change handler will navigate to the same route with different searchParams
And content from that path is fetched and server-side loaded onto the page

<script>
  import 'carbon-components-svelte/css/white.css'
  import {
    DataTable,
    Pagination
  } from 'carbon-components-svelte'
  import { goto } from '$app/navigation'
  import { page } from '$app/stores'

  export let data

  $: tablePage = $page.url.searchParams.get('page')
    ? parseInt($page.url.searchParams.get('page'), pageSize)
    : 1

  let pageSize = 25

  /**
   * changePage via pagination dropdown or prev/next buttons
   * @param e
   */
  const changePage = async e => {
    const url = new URL($page.url)
    url.searchParams.set('page', e.detail.page)

    await goto(`?${url.searchParams.toString()}`)
  }
</script>

<DataTable
  title="Pizzas around the world"
  headers={data.headers}
  rows={data.rows}
>

  <!-- top pagination -->
  <Pagination
    page={tablePage}
    {pageSize}
    totalItems={data.itemCount}
    pageSizeInputDisabled
    on:change={changePage}
  />
</DataTable>

<!-- bottom pagination -->
<Pagination
  page={tablePage}
  {pageSize}
  totalItems={data.itemCount}
  pageSizeInputDisabled
  on:change={changePage}
/>

Notice my on:change handler; with the changes in this PR, Pagination will dispatch a change event whenever the user clicks on the next button, previous button, or interacts with the page dropdown or pageSize dropdown.

It was necessary for me to update the logic for Carbon's Select component since it was dispatching change whenever the value updated internally. Without this adjustment to Select's logic, the user could click on the Next button, the Pagination component would update the page dropdown's state internally, the Select component would dispatch change, and my changePage handler would be called twice in a row after only 1 user interaction. According to MDN, the change event should dispatch during a user interaction, and the input event should dispatch whenever value changes. I want to capture the user change event after 1 user interaction.

[metonym]

I'll take another look at this. I'd like to couple this with #1518

[theetrain]

@metonym sounds good, let me know if you need any modifications.

[metonym]

I'm still not understanding the need to rename the dispatched event in Select from change to input. Especially since the Pagination component does not rely on the event from the change event anyway.

[theetrain]

I'm still not understanding the need to rename the dispatched event in Select from change to input. Especially since the Pagination component does not rely on the event from the change event anyway.

This PR enhances Pagination to forward a change event so that developers can listen to a change event anywhere within Pagination. This includes its PageSize and Page Selects that now forward change, and the 'Next' and 'Prev' Buttons that also forward change.

This is tricky for me to put into words succinctly, so here's a demo that hopefully achieves that 😃: https://stackblitz.com/edit/vitejs-vite-vsxhbs?file=src%2FApp.svelte

Since change represents a user action, we're only interested in listening and reacting to a user action. Previously, the Select component was not dispatching change, but was dispatching input anytime its internal value changes. This is problematic in a situation where there are 2 Pagination components on a page: for example, anytime a Select is changed on a Pagination toolbar at the top of the page, the other Pagination toolbar at the bottom of the page would dispatch input since it shares state with top Pagination toolbar, leading to two dispatches in a row even if there is only 1 user interaction.

The use case is to pass in the page prop without 2-way data binding, allowing control for client-side fetches to paginated data, and then displaying DataTable data in between 2 Pagination toolbars.

[theetrain]

I was a bit off base in my earlier comment. The main reason I am dispatching change from Select is for consistency. I want Pagination to respond to change events from Select so that there is assurance a user-controlled change had occurred in the component downstream.

The current Select component's forwarded input event dispatches based on Svelte variable reactivity, and not necessarily user interaction.

[kennethklee]

i'm hitting this too. I have a pagination element on the top and bottom of a datatable. and because page changes in one, it triggers update again at the bottom pagination element.

this is because the update event triggers for any programatic updates.

we need something that only triggers for user input -- which @theetrain is suggesting to dispatch a change event.

[kennethklee]

bump. the workaround for this is really hacky.

this PR is needed to align with the behavior of the change event in normal html select elements.

https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/change_event

[metonym]

I'm on board with this.

Three things:

• change also needs to be forwarded to the non-inline select
• moving forward, let's have dispatched event names not conflict with native names. How about changing the dispatched name from "input" to "update"?
• docs for Select need to be updated

[theetrain]

Updated, now update is dispatched instead of input. This may set an interesting precedent: do we want to forward native events and get their properties from event.target.value, or should we always use custom dispatchers?

This decision relates to my recent change to Dropdown in #1569.

[metonym]

Looks great!

Side note, it would be nice if the note I added to the @event JSDoc appeared on the rendered docs. Perhaps I can open a separate issue for that.

Non-blocking: event descriptions should now be supported by the recent sveld upgrade. If you rebase on master and re-run yarn; yarn build:docs, the Pagination event description for "change" should appear in generated docs metadata and types.

[theetrain]

I decided to scope creep a bit and modify the 'dispatched events' documentation.

Here's a preview for Select:

And for Pagination:

And for something like Checkbox that doesn't yet have descriptions:

[theetrain]

Released in v0.71.0