Skip to content

Conversation

@pareshpansuriya
Copy link
Contributor

Description
Earlier escapeHtmlAttr() is used instead of escapeHtml(). As it is simple message so we have replaced this method: escapeHtmlAttr() with escapeHtml().

  1. Default Welcome message is broken on storefront with enabled translate-inline #12711: Default Welcome message is broken on storefront with enabled translate-inline

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jan 7, 2018

CLA assistant check
All committers have signed the CLA.

@ishakhsuvarov
Copy link
Contributor

Hi @paresh1
Please sign the CLA so that we can proceed with the review and merge

@dmanners dmanners added the mm18in label Jan 7, 2018
@magento-engcom-team magento-engcom-team added bugfix mageconf Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release labels Jan 7, 2018
@dmanners dmanners removed the mageconf label Jan 7, 2018
@mzeis mzeis self-assigned this Jan 8, 2018
@mzeis mzeis added this to the January 2018 milestone Jan 8, 2018
@mzeis
Copy link
Contributor

mzeis commented Jan 22, 2018

Hi @pareshpansuriya,

thanks for your contribution! Tests showed that his PR fixes the problem for the blank theme but not for the Luma theme on the current commit e411c1d in branch 2.2-develop.

Please can you have a look at this? Thanks!

Before:

blank_before
luma_before

After:

blank_after
luma_after

@mzeis
Copy link
Contributor

mzeis commented Jan 22, 2018

@pareshpansuriya To be precise: you actually did the right thing and fixed the "Default welcome msg"! 👍 If you don't want to fix the second problem shown in the screenshots please tell.

@pareshpansuriya
Copy link
Contributor Author

pareshpansuriya commented Jan 23, 2018

I have try to solve that problem that create another problem when try to translate text "or" , that display individual whole text in popup for text : [{'shown':'or','translated':'or','original':'or','location':'Tag attribute (ALT, TITLE, etc.)'}]

Please check screen-shot.

So I am trying for solve that issue. Any help from you is appreciate.

screenshot from 2018-01-23 14-52-21

@mzeis
Copy link
Contributor

mzeis commented Jan 28, 2018

@pareshpansuriya Thanks for taking a look at this! I'm sorry but I'm not able to have a look at the "or" issue in the next days.

I suggest we get your fix for the original issue merged so your contribution can make it into a release soon. For the problem with "or", a separate issue could be created and then worked on. Is this ok for you?

@pareshpansuriya
Copy link
Contributor Author

Ok no problem I create pull request for that.

@pareshpansuriya
Copy link
Contributor Author

Thanks for do that, I actualy dont know how to edit PR for commit can you give any reference.

@mzeis
Copy link
Contributor

mzeis commented Feb 19, 2018

When you make a new commit to your branch and execute git push again, the PR is updated!

The problem from #13038 (comment) is still there with this commit 7e913f1, right?

@pareshpansuriya
Copy link
Contributor Author

Yes that is new problem with that commit, that commit have solved the previous issue #12711

@mzeis
Copy link
Contributor

mzeis commented Feb 27, 2018

@pareshpansuriya Ok. We will merge the "Welcome message" fix without a fix for the "or" problem. Thanks for your patience!

@mzeis
Copy link
Contributor

mzeis commented Mar 2, 2018

@pareshpansuriya Thanks again for your patience and your contribution! Your PR has been merged into 2.2-develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Progress: accept Release Line: 2.2 Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants