Refactor inline edit components

[alexkuc]

PR 1317

jump to support testing instructions

Scope

This is continuation of working on #1184

This PR prepares the card-related components to implement the functionality (separate PR)

Technical Details

There are 3 distinct layers for rendering UI when it comes to consumers of those components:

• Chakra-UI (3rd party library)
• Adapter package (EE package)
• UI package (EE package)

In order to facilitate display of decimals, we need to separate events onChange and onSubmit as currently onChange is treated as onSubmit. Current implementation does not provide us with ability to easily pass-through Chakra props to implement that. Adapters appear to be following a facade pattern i.e. expose only a limited set of props. This PR modifies InlineEdit component within Adapters package to allow passing any Chakra available prop without imposing the limit.

Additionally, I have explored the following dilemma… Any change to the existing packages results in "resonance cascade" i.e. all consumers of the affected packages become subject to testing. To alleviate work-related overload and to keep the scope of PRs strictly to their requirements I've implemented an adapter pattern for the props where consumers continue to pass old (legacy) style of props while at the same time accepting the style of props. This way, the testing scope becomes limited to "smoke test" only as opposed to have to re-test the entire functionality of the refactored component.

Testing Instructions

[!IMPORTANT]
The instructions below make the following assumptions:

• Cafe repository points to latest DEV branch
• Barista repository points to latest changes from this branch
• Plugin Event Espresso is active
• Plugin Barista for Event Espresso is active

Build

• git checkout MOD/refactor-inline-edit-components
• yarn install (just-in-case)
• yarn build (actual testing command)

Tested files

- domains/core/admin/eventEditor/src/ui/EventRegistrationOptions/types.ts - packages/adapters/src/InlineEdit/Adapter/index.ts - packages/adapters/src/InlineEdit/Adapter/types.ts - packages/adapters/src/InlineEdit/index.ts - packages/adapters/src/InlineEdit/InlineEdit/index.ts - packages/adapters/src/InlineEdit/InlineEdit/types.ts - packages/adapters/src/InlineEdit/Input/index.ts - packages/adapters/src/InlineEdit/Input/types.ts - packages/adapters/src/InlineEdit/legacy-types.ts - packages/adapters/src/InlineEdit/Preview/index.ts - packages/adapters/src/InlineEdit/Preview/types.ts - packages/adapters/src/InlineEdit/tsconfig.json - packages/adapters/src/InlineEdit/types.ts - packages/data/src/types.ts - packages/utils/src/text/index.ts - packages/utils/src/text/insertStrAt/index.ts

Component

[!IMPORTANT]
This PR is pure refactor so you experience anything that works differently than it used to, please report as a bug!

• create a new event and publish it
• change event name of the existing (default) datetime by editing its name inline
• to ensure that database was updated, please refresh the page and assert that changed name still appears
• repeat the same step for existing (default) ticket
• repeat these steps for new datetime and ticket
• change capacity of the existing (default) datetime and refresh page to ensure database was updated
• change ticket quantity of the existing (default) ticket and refresh page to ensure database was updated
• repeat these steps for a new datetime and ticket
• change ticket price of the existing (default) ticket by editing its total price inline
• refresh page to ensure database was updated
• repeat this step for a new ticket

Internal details

InlineEdit.tsx -> InlineEditInfinity.tsx -> eventEditor -> list -> cardView -> date capacity, ticket quantity ⚠️ InlineEdit.tsx -> InlineEditTextarea.tsx -> not in use ❎ Preview.tsx -> InlineEditInfinityPreview.tsx -> not in use ❎ Preview.tsx -> InlineEditText.tsx -> eventEditor -> list -> datetime name, ticket name ⚠️ Preview.tsx -> InlineEditText.tsx -> currency editor ⚠️ Preview.tsx -> InlineEditTextarea.tsx -> not in use ❎

Tested files

- packages/adapters/src/InlineEdit/Adapter/Adapter.tsx - packages/adapters/src/InlineEdit/Adapter/convertInputType.ts - packages/adapters/src/InlineEdit/Adapter/convertProps.ts - packages/adapters/src/InlineEdit/Adapter/createInputProp.ts - packages/adapters/src/InlineEdit/Editable.tsx - packages/adapters/src/InlineEdit/InlineEdit.tsx - packages/adapters/src/InlineEdit/InlineEdit/InlineEdit.tsx - packages/adapters/src/InlineEdit/InlineEditInput.tsx - packages/adapters/src/InlineEdit/InlineEditPreview.tsx - packages/adapters/src/InlineEdit/Input/Input.tsx - packages/adapters/src/InlineEdit/Input/onKeyDown.ts - packages/adapters/src/InlineEdit/Preview/convertToLegacyProps.ts - packages/adapters/src/InlineEdit/Preview/Preview.tsx - packages/ui-components/src/InlineEdit/InlineEditInput/InlineEdit.tsx - packages/ui-components/src/InlineEdit/InlineEditInput/Preview.tsx - packages/utils/src/text/insertStrAt/index.test.ts - packages/utils/src/text/insertStrAt/insertStrAt.ts

[alexkuc]

Local Testing

• [x] run linter ✅
• [x] run build ✅
• [x] run unit tests ✅
• [x] run e2e tests ✅

~could not test PayPal-related tests due to failure of its activation (new error)~

Edit: e2e has now fully passed! ✅

[alexkuc]

/run-e2e

https://github.com/eventespresso/barista/actions/runs/9667328108 ❌

[alexkuc]

Need to rebase to use Node v20 version

[alexkuc]

Rebasing…

Old branch is MOD/refactor-tpc-price-fields (7dfa507). Old head is 593118a

New branch is master (6483ed5). New head is f51b8e8.

[alexkuc]

/run-e2e

https://github.com/eventespresso/barista/actions/runs/9686409254 ✅

[tn3rb]

Adapters appear to be following a facade pattern i.e. expose only a limited set of props.

Adapters follow the Adapter pattern 😆

from https://refactoring.guru/design-patterns/adapter

Adapter is a structural design pattern that allows objects with incompatible interfaces to collaborate.

The idea is to NOT use Chakra UI components (or any third party components) directly but instead ALWAYS use an adapter and only expose the implementation details we need in an attempt to avoid tight coupling. If we ever need to change a component(s) from Chakra to some other third party library, then we can do so by simply changing the adapter and hopefully leave all other code in place.

This has already been done successfully. When we first started building the EDTR, we were using the Ant Designs component library and I insisted on using adapters. The other devs thought i was crazy and that it was a waste of time. Over time we ran into more and more problems with accessibility because the Ant Designs library has seemingly made zero effort to make their components accessible. Eventually we got fed up and switched to Chakra UI. It basically took one days work for one dev with one PR to convert the EDTR from Ant Designs to Chakra UI (ignoring a few obscrure fixes that popped up afterwards).

[alexkuc]

I would have to disagree on the definition of adapter pattern here and how it is implemented/used.

Adapter is a structural design pattern that allows objects with incompatible interfaces to collaborate.

There is nothing incompatible here about business decision to abstract HTML interfaces to avoid relying on vendor-specific APIs. Image taken from the link above:

If we had two entirely different libraries, frameworks, APIs, data structures, etc. we would need an adapter to adapt data from one type/structure to another.

On the other hand, taking definition of the problem facade pattern aims to solve:

Problem ... As a result, the business logic of your classes would become tightly coupled to the implementation details of 3rd-party classes, making it hard to comprehend and maintain.

To quote your reply, imho, fits perfectly with the definition of the problem above

The idea is to NOT use Chakra UI components (or any third party components) directly but instead ALWAYS use an adapter and only expose the implementation details we need in an attempt to avoid tight coupling.

Edit: even if we disagree on the definitions above, the naming itself is immaterial to me, what's important for me to understand is the logic/business decision behind so I can actually follow it

[alexkuc]

With this in mind, I'll close this PR and re-do it following the business decision above.