-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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.
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
fix(item): use thumbnail's size when present #27014
Conversation
Run & review this pull request in StackBlitz Codeflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look great! Could we add a test so this does not regress?
Maybe something like this:
test('size should be customizable in item', async ({ page, skip }) => {
skip.rtl();
await page.setContent(`
<ion-item>
<ion-thumbnail style="--size: 20px">
<img src="/src/components/thumbnail/test/thumbnail.svg" />
</ion-thumbnail>
</ion-item>
`);
const item = page.locator('ion-item');
await expect(item).toHaveScreenshot(`item-thumbnail-size-${page.getSnapshotSettings()}.png`);
});
We will want to test iOS and MD mode since we are updating the code for both modes. However, we can skip the LTR vs RTL tests. The focus of this test should be ensuring that developers can use the --size
API on ion-thumbnail
to customize its size even when used inside of ion-item
.
edit:
You can also add this inside of the test body:
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/22935',
});
This adds a Custom Playwright Annotation to indicate that this test was added to verify a particular bug fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Pull request checklist
Please check if your PR fulfills the following requirements:
ionic-docs
repo, in a separate PR. See the contributing guide for details.npm run build
) was run locally and any changes were pushednpm run lint
) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
The
--size
fromion-thumbnail
is being overwritten when component is placed insideion-item
byion-item
's slotted styling.This contradicts the behavior described in the
ion-thumbnail
documentation.Issue URL: resolves #22935
What is the new behavior?
ion-item
will set a default value forion-thumbnail
's--size
when presentPasses a default size to thumbnail when slotted.
Does this introduce a breaking change?
Other information
N/A