Skip to content

test(tab-bar): add snapshot for box shadow #30134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jan 17, 2025
Merged

test(tab-bar): add snapshot for box shadow #30134

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a30ba24
Making tab bar box shadow independent of expand mode
OS-pedrolourenco Jan 8, 2025
b01d817
Fix lint issue
OS-pedrolourenco Jan 8, 2025
d60840e
Merge branch 'next' into ROU-11503
OS-pedrolourenco Jan 9, 2025
6111c17
Adding snapshot tests for shadow feature
OS-pedrolourenco Jan 14, 2025
8a7836b
Merge branch 'next' into ROU-11503
OS-pedrolourenco Jan 14, 2025
8acd903
Merge branch 'ROU-11503' into ROU-11503-snapshot-tests
OS-pedrolourenco Jan 14, 2025
9b2bcda
CR
OS-pedrolourenco Jan 15, 2025
522aafa
CR
OS-pedrolourenco Jan 16, 2025
1d1e01c
test(tab-bar): use correct code syntax
thetaPC Jan 16, 2025
0ec4c2d
CR from @brandyscarney
OS-pedrolourenco Jan 17, 2025
587a728
Merge branch 'next' into ROU-11503-snapshot-tests
OS-pedrolourenco Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 60 additions & 18 deletions core/src/components/tab-bar/test/expand/tab-bar.e2e.ts
View file
Open in desktop
Copy link
Contributor

@thetaPC thetaPC Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snapshots should always aim to be as small as possible. That's why the original test was using tabBar instead of content. Since we want to capture the box shadow, then create a container that has padding and use the container for the snapshot instead for both tests. Here's some pseudo code:

.container {
   padding: 20px 10px;
}

<ion-content>
   <div class="container">
      <ion-tab-bar>
   </div>
</ion-content>

const container = page.locator('.container');

await expect(container).toHaveScreenshot...

Copy link
Contributor Author

@OS-pedrolourenco OS-pedrolourenco Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The compact snapshots look a bit strange though.
By the way, I ended up not using the container in your suggestion seen as though it had no effect on the snapshots.

Copy link
Contributor

@thetaPC thetaPC Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because I used pseudo code as in my code is only semi complete so:

  1. Please use the proper syntax
  2. Add the suggested code to the correct locations
  3. use the container
.container {
   padding: 20px 10px;
}

<ion-content>
   <div class="container">
      <ion-tab-bar> // not complete, use proper syntax
   </div>
</ion-content>

const container = page.locator('.container');

await expect(container).toHaveScreenshot... // not complete, use proper syntax

Copy link
Contributor Author

@OS-pedrolourenco OS-pedrolourenco Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope I have addressed your concern with my latest commit. The only adjustment I had to do was to manually set the container dimensions on the compact tab bar test since, with only the padding attribute set up, the container shown in the snapshot did not fit the tab bar for some reason and was hiding it instead:
image
If you have any suggestion for better working around this, please let me know.

Copy link
Contributor

@thetaPC thetaPC Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and made the appropriate changes since it seems that my comments regarding using proper syntax is not being understood well. I would highly recommend reviewing my changes since it showcases how tests should be written for Ionic. Thanks!

Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,45 @@ import { configs, test } from '@utils/test/playwright';
*/
configs({ directions: ['ltr'], modes: ['ionic-md'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('tab-bar: expand'), () => {
test.describe(title('full'), () => {
test('should not have visual regressions', async ({ page }) => {
await page.setContent(
`
<style>
:root {
background: #ccc7c7;
}
</style>

<ion-content>
<ion-tab-bar expand="full">
<ion-tab-button tab="1">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="2">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="3">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>
</ion-tab-bar>
</ion-content>
`,
config
);

// Used the `ion-content` element to take the screenshot because the `ion-tab-bar`element would not be visible otherwise
const content = page.locator('ion-content');

await expect(content).toHaveScreenshot(screenshot(`tab-bar-expand-full`));
});
});

test.describe(title('compact'), () => {
test('should not have visual regressions', async ({ page }) => {
await page.setContent(
Expand All @@ -16,29 +55,32 @@ configs({ directions: ['ltr'], modes: ['ionic-md'] }).forEach(({ title, screensh
}
</style>

<ion-tab-bar expand="compact">
<ion-tab-button tab="1">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="2">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="3">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>
</ion-tab-bar>
<ion-content>
Copy link
Member

@brandyscarney brandyscarney Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove ion-content as this is not needed

<ion-tab-bar expand="compact">
<ion-tab-button tab="1">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="2">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>

<ion-tab-button tab="3">
<ion-icon name="triangle-outline"></ion-icon>
<ion-label>Label</ion-label>
</ion-tab-button>
</ion-tab-bar>
</ion-content>
`,
config
);

const tabBar = page.locator('ion-tab-bar');
// Used the `ion-content` element to take the screenshot because the `ion-tab-bar`element would not be visible otherwise
const content = page.locator('ion-content');

await expect(tabBar).toHaveScreenshot(screenshot(`tab-bar-expand-compact`));
await expect(content).toHaveScreenshot(screenshot(`tab-bar-expand-compact`));
});
});
});
Expand Down
Binary file modified ....ts-snapshots/tab-bar-expand-compact-ionic-md-ltr-light-Mobile-Chrome-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...ts-snapshots/tab-bar-expand-compact-ionic-md-ltr-light-Mobile-Firefox-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ....ts-snapshots/tab-bar-expand-compact-ionic-md-ltr-light-Mobile-Safari-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...e2e.ts-snapshots/tab-bar-expand-full-ionic-md-ltr-light-Mobile-Chrome-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...2e.ts-snapshots/tab-bar-expand-full-ionic-md-ltr-light-Mobile-Firefox-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...e2e.ts-snapshots/tab-bar-expand-full-ionic-md-ltr-light-Mobile-Safari-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading