chore(playwright): combine test configs for themes and modes

[sean-perkins]

Issue number: N/A

---

What is the current behavior?

Developers are not able to easily add the "ionic" theme to existing test cases, without running the test against both iOS and MD mode:

configs({ themes: ['ios', 'md', 'ionic'] })
// Generates 4 test cases
// - iOS theme on iOS mode
// - MD theme on MD mode
// - Ionic theme on iOS mode
// - Ionic theme on MD mode

With the separation of mode into look and feel, the majority of test cases do not require testing the mode behavior and instead only need to test the visual theme that is applied to the component.

What is the new behavior?

• Removes the themes option from the configs test generator object.

configs({ modes: ['ios', 'md', 'ionic-md'] })

• Combines theme and mode into the existing modes test generator object
  • The new options are ionic-ios and ionic-md, to run the Ionic theme against the respective mode.
  • This path was preferred to avoid deprecating and migrating all existing tests.

Does this introduce a breaking change?

• Yes
• No

Other information

I do not have a strong preference on the semantics of ionic-ios vs. ios-ionic (theme first vs. mode first). If anyone has an opinion or alternative suggestion, please add a comment.

[mapsandapps]

One idea for a comment. Implementation is good 👍🏻

[mapsandapps]

I know we have a comment elsewhere, but it might be nice to note here that we're ignoring the md-ios and ios-md combos for testing.

[liamdebeasi]

+1. Also good to document why we are ignoring those combos

[liamdebeasi]

I think we can simplify this (GitHub won't let me make a code suggestion because it would go across deleted lines)

processedModes.forEach((mode) => {
    processedDirection.forEach((direction) => {
      processedPalette.forEach((palette) => {
        const [theme, modeName] = mode.split('-');
        configs.push({ direction, palette, mode: (modeName ?? theme) as Mode, theme: theme as Theme });
      });
    });
  });

In both cases we need to iterate over directions and palettes, so having separate code paths here doesn't really save us much work. For the 'ios' and 'md' cases, modeName will be undefined which is why we fall back to theme setting the mode key in the config object.

[liamdebeasi]

There's probably a way to simplify the types, but I didn't spend too much time thinking about that part.

[sean-perkins]

I took a slightly different approach, curious what you think: eb9f5d6.

I don't think we should suggest a theme can be a mode: (modeName ?? theme) as Mode. A mode can be a theme (for backwards compatibility), but a theme should not be a mode.

[liamdebeasi]

The main problem here is we don't know if mode contains either a theme and a mode or a combined mode string ahead of time, so the naming in my original approach is confusing.

What you have works, but it's doing an additional indexOf check which will iterate through the mode input more than we need to.

Perhaps we can have something like this:

const [themeOrCombinedMode, modeName]  = mode.split('-');

const parsedTheme = themeOrCombinedMode as Theme;

// If modeName is undefined then we have a combined mode string, so the first variable is actually the mode
const parsedMode = modeName ?? themeOrCombinedMode as Mode;

This approach avoids the additional iteration through the string.