commercetools / ui-kit

Component library 💅
https://uikit.commercetools.com
MIT License
145 stars 25 forks source link

ui-kit data-table-manager component there is no prop to add a click handler to the `x` button #2244

Open katstacyryan opened 2 years ago

katstacyryan commented 2 years ago

The Merchant Center >Audit Log> Change History user interface allows users to change their table settings for layout settings and manage the columns visible in a results grid. When the user selects either the layout settings option or the column manager option, they can select changes they are thinking of applying. The user may decide not to apply these changes. They select the X function in the top right to close the table settings. This is the same as canceling. However, the X function doesn't allow for a refresh. This means the changes the user selected in the table settings were applied even though they backed out of their decision by selecting the X.

There are two Jira tickets created by the Pangolin team that show the steps to reproduce. https://jira.commercetools.com/browse/PANGOLIN-1833 has a screen recording attached to show the behavior https://jira.commercetools.com/browse/PANGOLIN-1834

Describe the bug In the ui-kit data-table-manager component there is no prop to add a click handler to the x button, so there is no way to change this behavior on the audit-log side. Please update the component with a prop to add a click handler on the X button. The missing functionality causes defective behavior for end users in the audit log implementation of this component. The X button to close the merchant center, audit log, table settings does not refresh and therefore selected options are applied on the results grid when they should not be.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'Merchant Center > Audit Log > Change history'
  2. Look for a drop down menu on the right side of the screen above the results gird called "Table Settings"
  3. Select either Layout settings or Column Manager , each one implements the data-table-manager component
  4. Make changes to the settings
  5. Select the X function in the top right of the component
  6. See that the results are drawing as if the last selections were saved/applied, but that is not the expected behavior.

Expected behavior When the user selects X in the table settings, that means they are canceling and do not want to apply these selections to the results grid. The results grid should refresh back to the state it was in prior to the user experimenting with the table settings.

Screenshots See the jira tickets. https://jira.commercetools.com/browse/PANGOLIN-1833 has a screen recording attached to show the behavior https://jira.commercetools.com/browse/PANGOLIN-1834

Rhotimee commented 2 years ago

Hello @katstacyryan, thank you for opening this issue. We will get back to you on this soon.

mmaltsev-ct commented 2 years ago

Hi @ByronDWall @FilPob, as discussed, let's try to collect here as much information about this use case as possible. Please drop here a message after doing the discovery regarding the current implementation behind Save and Cancel buttons, and after talking to Diane regarding the expected UX for using the table settings in Audit Log.

ByronDWall commented 2 years ago

Audit log's workflow for table settings relies on formik. All the column values and settings values are stored there in the initial formik state.

As the user updates values in the columns or settings manager, they are updating formik state values, and updating the UI based on those state updates "automatically".

When the user hits save, the column/table settings are persisted to localStorage via a callback, which is how the changes are persisted between sessions (and set as the initial formik state in a new session).

When the user hits cancel, the formik state is reset via formik.handleReset, which is how the changes are reverted visually. Because we cannot pass formik.handleReset to the x in the DataTableManager, there is no way we are able to reset the form if the user closes it without hitting the x.

One of the things this means for our specific usecase is that, if a click handler were added to the x, there does not need to be a separate handler for 'column settings' and 'table settings'. An onCancel/onDismiss/onClose prop would work as well.