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

Fix QA feedbacks #414

Merged
merged 6 commits into from
Jan 5, 2023
Merged

Fix QA feedbacks #414

merged 6 commits into from
Jan 5, 2023

Conversation

NeOMakinG
Copy link
Contributor

Questions Answers
Description? The QA did some feedbacks that we need to tackle
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #409 .
How to test? See QA feedbacks list of the issue and test it
Possible impacts? Global

@SharakPL
Copy link
Contributor

SharakPL commented Dec 21, 2022

Do we need XHTML compatibility?
I planned to get rid of /> from all img input br hr etc.

@NeOMakinG
Copy link
Contributor Author

Do we need XHTML compatibility? I planned to get rid off /> from all img input br hr etc.

Well it was to check for a browser validation bug but we can just kick them out

@SharakPL
Copy link
Contributor

We could use a smarty linter/fixer to enforce consistency through all themes and modules: remove useless />, tags' attributes inline or one per row, etc. But honestly I don't know anything useful for that. I use VSCode and Smarty extension as a formatter, but it's still not perfect.

@NeOMakinG
Copy link
Contributor Author

We could use a smarty linter/fixer to enforce consistency through all themes and modules: remove useless />, tags' attributes inline or one per row, etc. But honestly I don't know anything useful for that. I use VSCode and Smarty extension as a formatter, but it's still not perfect.

Yeah, I did try to find one years ago but unfortunately, it doesn't seem to exist at all 😢

@NeOMakinG NeOMakinG requested review from Hlavtox, SharakPL and a team December 23, 2022 10:19
@NeOMakinG NeOMakinG marked this pull request as ready for review December 23, 2022 10:19
src/scss/core/components/_search.scss Outdated Show resolved Hide resolved
src/scss/core/layout/_header.scss Outdated Show resolved Hide resolved
templates/_partials/form-fields.tpl Outdated Show resolved Hide resolved
templates/_partials/form-fields.tpl Outdated Show resolved Hide resolved
templates/customer/_partials/order-messages.tpl Outdated Show resolved Hide resolved
@@ -45,7 +45,7 @@
{/block}

{block name='cart_voucher_notifications'}
<div class="alert alert-danger js-error d-none" role="alert">
<div class="alert alert-danger js-error mt-2" role="alert" style="display: none;">
Copy link
Contributor

Choose a reason for hiding this comment

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

Inline style? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kpodemski this is a core limitation from the core.js file toggling this error when the voucher trigger it

@Hlavtox Hlavtox merged commit cc4e596 into PrestaShop:develop Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QA feedbacks - bug list
4 participants