Skip to content
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

[Icons] Fix LockIconsCommand definition when Iconify disabled #2416

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Dec 2, 2024

Q A
Bug fix? yes
New feature? no
Issues Fix #2415
License MIT

When iconify is disabled, LockIconsCommand definition is accessed without been set first.

@carsonbot carsonbot added Bug Bug Fix Icons Status: Needs Review Needs to be reviewed labels Dec 2, 2024
@smnandre smnandre requested review from Kocal and kbond and removed request for Kocal December 2, 2024 03:56
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Dec 2, 2024
@Kocal
Copy link
Member

Kocal commented Dec 2, 2024

Thanks for your reactivity Simon, I didn't see anything wrong while merging #2410 😅

It would be really nice to have E2E/smoke tests that will create a new Symfony app, install the component, and see what happens

@smnandre
Copy link
Member Author

smnandre commented Dec 2, 2024

Thanks for your reactivity Simon, I didn't see anything wrong while merging #2410 😅

It would be really nice to have E2E/smoke tests that will create a new Symfony app, install the component, and see what happens

This one is 100% on me, did not check enough when i updated the extension.

@smnandre
Copy link
Member Author

smnandre commented Dec 2, 2024

It would be really nice to have E2E/smoke tests that will create a new Symfony app, install the component, and see what happens

This would not have detected the problem here, as you need to disable iconify to trigger it.

But that mean we are far from having enough fonctional test.

@javiereguiluz javiereguiluz force-pushed the fix/icon-loc-definition branch from 7fce7d8 to 6ba36f7 Compare December 2, 2024 16:45
@javiereguiluz
Copy link
Member

Thanks Simon.

@javiereguiluz javiereguiluz merged commit ec4cfa1 into symfony:2.x Dec 2, 2024
8 checks passed
@trsteel88
Copy link

@javiereguiluz is it possible to get this tagged? It's currently breaking all our applications.

Thanks.

@Kocal
Copy link
Member

Kocal commented Dec 2, 2024

@trsteel88 you can pin symfony/ux-icons at version 2.21.0 for the moment.

@kbond ideally we want to tag a new release after #2420 too 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Icons Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Icons] Console error after update
5 participants