search

ipeychev / liferay-aui-upgrade-tool

1 stars 1 forks source link

Improper replacement of portlet-msg-info in css files #3

Closed robframpton closed 11 years ago

robframpton commented 11 years ago

UT will always change '.portlet-msg-info' in css files into '.alert .alert-info'

.portlet-msg-info { background-color: transparent; }

.alert .alert-info { background-color: transparent; }

However this block will not actually select anything, as .alert and .alert-info are classes of the same element, .alert-info is not a child of .alert. The selector should be changed to either

.alert.alert-info { background-color: transparent; }

or

.alert-info { background-color: transparent; }

robframpton commented 11 years ago

Same thing happens with all alert classes, such as portlet-msg-error.

ipeychev commented 11 years ago

Robert, these classes are configurable in: assets/css-classes.json

Would you mind to rename these to:

"portlet-msg-error": "alert-error",
"portlet-msg-info": "alert-info",
"portlet-msg-success": "alert-success"

And tell me if that fixes the issue?

ipeychev commented 11 years ago

Also, is that issue valid only for CSS files, or it is valid for both CSS and JSP files?

robframpton commented 11 years ago

The issue only exists within the JSP files. Changing to is correct, the issue is that it's doing the same thing in the CSS files.

So in JSPs, 'portlet-msg-error' = 'alert alert-error' is good

But in CSS, '.portlet-msg-error' = '.alert .alert-error' is bad

robframpton commented 11 years ago

Hey Iliyan,

This fix does not work, as now in JSPs, 'portlet-msg-error' will be changed to 'alert-error' instead of 'alert alert-error'. We need it to do two different things depending on the file type. So this fix works for CSS files, but breaks it for JSPs, the way it is now in master works for JSPs, but is broken for CSS.

On Thu, Aug 22, 2013 at 11:02 AM, Robert Frampton < robert.frampton@liferay.com> wrote:

Yes I'll give it a shot and get back to you.

On Thu, Aug 22, 2013 at 10:19 AM, Iliyan Peychev <notifications@github.com

wrote:

Robert, these classes are configurable in: assets/css-classes.json

Would you mind to rename these to:

"portlet-msg-error": "alert-error","portlet-msg-info": "alert-info","portlet-msg-success": "alert-success"

And tell me if that fixes the issue?

— Reply to this email directly or view it on GitHubhttps://github.com/ipeychev/upgrade-tool/issues/3#issuecomment-23109212 .

robframpton commented 11 years ago

Yes I'll give it a shot and get back to you.

On Thu, Aug 22, 2013 at 10:19 AM, Iliyan Peychev notifications@github.comwrote:

Robert, these classes are configurable in: assets/css-classes.json

Would you mind to rename these to:

"portlet-msg-error": "alert-error","portlet-msg-info": "alert-info","portlet-msg-success": "alert-success"

And tell me if that fixes the issue?

— Reply to this email directly or view it on GitHubhttps://github.com/ipeychev/upgrade-tool/issues/3#issuecomment-23109212 .

ipeychev commented 11 years ago

One correction - the issue is related to classes, used as selectors. These might be in JSP files too, for example:

A.all('.portlet-msg-success').hide();

We have to change these too, like:

.alert.alert-info