Skip to content

Conversation

@OstafinL
Copy link
Contributor

@OstafinL OstafinL commented Oct 27, 2025

@OstafinL OstafinL force-pushed the IBX-10555-added-blured-loader-for-modal branch from 9a9e304 to ac95ed9 Compare October 27, 2025 11:02
@OstafinL OstafinL force-pushed the IBX-10555-added-blured-loader-for-modal branch 3 times, most recently from bc2866a to b2f4d20 Compare October 27, 2025 11:20
@OstafinL OstafinL changed the title IBX-10555: Added blured loader for modal IBX-10555: Added blurred loader for modal Oct 27, 2025
@OstafinL OstafinL requested a review from a team October 27, 2025 13:20
@ezrobot ezrobot requested review from GrabowskiM, RopRaptor, albozek, alekmick, dew326 and tischsoic and removed request for a team October 27, 2025 13:21
};

export { controlZIndex, controlManyZIndexes };
const showModalLoader = ({ headerText, descriptionText, modalNode }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

personally I would put modalNode as its own method argument, as it's THE element which is mandatory for it to even work?
Same with hideModalLoader (there it would be even more useful, as there would be no need to pass object just to pass one argument

Comment on lines 78 to 80
&__loader-header-text {
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&__loader-header-text {
}

Comment on lines 78 to 79
&__loader-header-text {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&__loader-header-text {
}

modalNode.classList.remove('ibexa-modal--with-blurred-loader');
};

document.body.addEventListener('hidden.bs.modal', (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Own concern.

Seems like this might affect ALL modals on the page - is that intended?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is intentional, but it should have checked if the modal was blurred and unsage of helper should be in different place, not in helper itself because we can have helper imported multiple time on the page which can cause some errors.

modalNode.classList.remove('ibexa-modal--with-blurred-loader');
};

document.body.addEventListener('hidden.bs.modal', (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is intentional, but it should have checked if the modal was blurred and unsage of helper should be in different place, not in helper itself because we can have helper imported multiple time on the page which can cause some errors.

@juskora juskora force-pushed the IBX-10555-added-blured-loader-for-modal branch from b930c36 to 4476813 Compare November 3, 2025 09:27
@juskora
Copy link
Contributor

juskora commented Nov 4, 2025

@juskora juskora force-pushed the IBX-10555-added-blured-loader-for-modal branch from 70844c8 to 88df1bf Compare November 12, 2025 08:04
@sonarqubecloud
Copy link

@juskora
Copy link
Contributor

juskora commented Nov 12, 2025

QA Approved on Ibexa DXP Commerce 4.6-dev.

@dew326 dew326 merged commit 4371e66 into 4.6 Nov 13, 2025
28 checks passed
@dew326 dew326 deleted the IBX-10555-added-blured-loader-for-modal branch November 13, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants