[Branch ~dhis2-devs-core/dhis2/trunk] Rev 191: blueprints:ie6-compliance, low-res. Other js fixes for all browsers... Author: Zafar, Bharath, Sei...

[Lars (Lars Øverland)]

Hi,

nice work with the ie6 compliance. I have a few humble comments:

• You have only done updates for the blue skin. This makes things inconsistent, please update the rest.

• Why have you changed logo_banner.png? In my browser this is no longer vertically centered.

• Why did you remove the moveAll function? Have you made sure it is not used at all in the system?

• Even if you have changed locking.js, this type of functionality can be found in lots of places in the system which makes things inconsistent. Also I thought this part worked in IE6? Can we have it the way it was?

• The right side padding of the logo is less than the left side padding, can you make this even?

• You are using a different code style then the system default. I am not saying your style is bad, but the point of having a codestyle is that things should look the same, now we have different code styles which is ugly.

Lars

···

On Sat, Apr 18, 2009 at 1:13 AM, noreply@launchpad.net wrote:

---

revno: 191

committer: sunbiz sunbiz@hispindia

branch nick: trunk

timestamp: Sat 2009-04-18 04:43:07 +0530

message:

blueprints:ie6-compliance,low-res. Other js fixes for all browsers… Author: Zafar, Bharath, Seid - new India theme part of v1 Indian deployment

added:

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/dom.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/head.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/india.css

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_background.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_left.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_right.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_separator.png

modified:

dhis-2/dhis-options/src/main/resources/META-INF/dhis/beans.xml

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/blue.css

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/logo_right.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/security/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/util/lists.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/main.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/request.js

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/javascript/locking.js

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/lockingForm.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/menu.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-datadictionary/src/main/webapp/dhis-web-maintenance-datadictionary/dataElementGroupEditor.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-datadictionary/src/main/webapp/dhis-web-maintenance-datadictionary/javascript/dataElementGroupEditor.js

The size of the diff (2263 lines) is larger than your specified limit of 1000 lines

–

Trunk

https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk

Your team DHIS 2 developers is subscribed to branch lp:dhis2.

To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription.

---

Mailing list: https://launchpad.net/~dhis2-devs

Post to : dhis2-devs@lists.launchpad.net

Unsubscribe : https://launchpad.net/~dhis2-devs

More help : https://help.launchpad.net/ListHelp

[sunbiz (Saptarshi Purkayastha)]

Hi lars,

• We are using the Indian theme and blue is by default… Will do it for the other themes in parts. Looked at solving the Indian needs at first ;-)… will now move to the next ones.

• The logo_banner.png was changed because instead of using a JavaScript hack to get the png in IE, I felt it was better to remove the transparency and use the same background color.

• The moveAll function did not work in IE6 and instead all the calls were through moveAllByID. Jquery does it in a single line and hence, didn’t see the use of moveAll function… If anyone’s module is affected (which I couldn’t find), I suggest that you may want to use moveAllByID.

• The functionality of locking.js was not working in IE6 as it is and hence the changes had to be made

• The padding is different because the application name gets into the maintainance text at the lower resolution and hence the right padding had to be smaller. And don’t know why the paddings have to be same on both sides?? Some bias to symmetry??

• Exactly my question, do we have a code style file that I can use with my IDE?? I couldn’t find it uploaded in launchpad… Someone can post the codestyle file in the Downloads section.

BTW, isn’t it a common norm in the Java world to have the { (opening brackets) on the same line as method instead of newline??

on the blueprint I added “Needs Code Review”, so that people can comment like this. I hope others respond with their views as well… Other people who are working on blueprints should also use this status on blueprints, so that we can have better code quality.

···

---

Regards,
Saptarshi PURKAYASTHA
Director R & D, HISP India
Health Information Systems Programme

My Tech Blog: http://sunnytalkstech.blogspot.com

You Live by CHOICE, Not by CHANCE

2009/4/18 Lars Helge Øverland larshelge@gmail.com

Hi,

nice work with the ie6 compliance. I have a few humble comments:

• You have only done updates for the blue skin. This makes things inconsistent, please update the rest.

• Why have you changed logo_banner.png? In my browser this is no longer vertically centered.

• Why did you remove the moveAll function? Have you made sure it is not used at all in the system?

• Even if you have changed locking.js, this type of functionality can be found in lots of places in the system which makes things inconsistent. Also I thought this part worked in IE6? Can we have it the way it was?

• The right side padding of the logo is less than the left side padding, can you make this even?

• You are using a different code style then the system default. I am not saying your style is bad, but the point of having a codestyle is that things should look the same, now we have different code styles which is ugly.

Lars

On Sat, Apr 18, 2009 at 1:13 AM, noreply@launchpad.net wrote:

---

revno: 191

committer: sunbiz sunbiz@hispindia

branch nick: trunk

timestamp: Sat 2009-04-18 04:43:07 +0530

message:

blueprints:ie6-compliance,low-res. Other js fixes for all browsers… Author: Zafar, Bharath, Seid - new India theme part of v1 Indian deployment

added:

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/dom.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/head.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/india.css

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_background.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_left.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_right.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/india/logo_separator.png

modified:

dhis-2/dhis-options/src/main/resources/META-INF/dhis/beans.xml

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/blue.css

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/css/blue/logo_right.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/security/logo_banner.png

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/util/lists.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/main.js

dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/request.js

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/javascript/locking.js

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/lockingForm.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataadmin/src/main/webapp/dhis-web-maintenance-dataadmin/menu.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-datadictionary/src/main/webapp/dhis-web-maintenance-datadictionary/dataElementGroupEditor.vm

dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-datadictionary/src/main/webapp/dhis-web-maintenance-datadictionary/javascript/dataElementGroupEditor.js

The size of the diff (2263 lines) is larger than your specified limit of 1000 lines

–

Trunk

https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk

Your team DHIS 2 developers is subscribed to branch lp:dhis2.

To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription.

---

Mailing list: https://launchpad.net/~dhis2-devs

Post to : dhis2-devs@lists.launchpad.net

Unsubscribe : https://launchpad.net/~dhis2-devs

More help : https://help.launchpad.net/ListHelp

---

Mailing list: https://launchpad.net/~dhis2-devs

Post to : dhis2-devs@lists.launchpad.net

Unsubscribe : https://launchpad.net/~dhis2-devs

More help : https://help.launchpad.net/ListHelp

[Lars (Lars Øverland)]

Hi Saptarshi,

thanks for commenting…

Hi lars,

• We are using the Indian theme and blue is by default… Will do it for the other themes in parts. Looked at solving the Indian needs at first ;-)… will now move to the next ones.

OK great no prob.

• The logo_banner.png was changed because instead of using a JavaScript hack to get the png in IE, I felt it was better to remove the transparency and use the same background color.

Hmm… It is nice to have these transparent PNGs, and way more flexible than merging in the background. This would require us to do this for all skins as well. If we want to change the logo it’s better to be able to do it one place. MS just released IE8, we shouldn’t put TOO much effort in serving IE6. I would very much like to have it the way it was…

• The moveAll function did not work in IE6 and instead all the calls were through moveAllByID. Jquery does it in a single line and hence, didn’t see the use of moveAll function… If anyone’s module is affected (which I couldn’t find), I suggest that you may want to use moveAllByID.

Sure I see. I’m just saying its a little dangerous to remove things from the common javascript files as we are not getting errors until runtime if things are broken. Maybe a grep on the code base would be nice here. Can’t think of any places it is used though.

• The functionality of locking.js was not working in IE6 as it is and hence the changes had to be made

OK fine. But as far as I know there are other places with similar code (datamart, report table etc), is this working?

• The padding is different because the application name gets into the maintainance text at the lower resolution and hence the right padding had to be smaller. And don’t know why the paddings have to be same on both sides?? Some bias to symmetry??

Well I believe most web-pages adhere to this, and I think it looks much better, yes…

• Exactly my question, do we have a code style file that I can use with my IDE?? I couldn’t find it uploaded in launchpad… Someone can post the codestyle file in the Downloads section.

You can find it here: http://208.76.222.114/confluence/display/DOC/Code+Conventions (on the middle of the page)

BTW, isn’t it a common norm in the Java world to have the { (opening brackets) on the same line as method instead of newline??

Might be, but all javascripts in the system are written similar to the java code style, and it looks weird with different styles. If there are strong arguments for changing the style that is fine with me, but then everything should be changed.

on the blueprint I added “Needs Code Review”, so that people can comment like this. I hope others respond with their views as well… Other people who are working on blueprints should also use this status on blueprints, so that we can have better code quality.

Yes this is very useful. Also, make sure we set blueprints to “started” so we avoid duplication of work and can get a view of what people are working on.

···

On Sat, Apr 18, 2009 at 1:11 PM, Saptarshi Purkayastha sunbiz@gmail.com wrote:

[sunbiz (Saptarshi Purkayastha)]

Lars,

Hmm… It is nice to have these transparent PNGs, and way more flexible than merging in the background. This would require us to do this for all skins as well. If we want to change the logo it’s better to be able to do it one place. MS just released IE8, we shouldn’t put TOO much effort in serving IE6. I would very much like to have it the way it was…

I wouldn’t believe the necessity for IE6 if I had not seen it with my eyes… A training session in HP made me realize that it can be difficult for people to ask them to download Firefox and XP comes default with IE6. They won’t get updates without the internet and everywhere vanilla XP without any service packs… Thats what motivated me to get this fix done asap.

I tried JavaScript to get the png and some CSS hacks… but it made some parts slow and hence removed the transparency… and we aren’t really re-using most images (eg. different images in login.html and header)

Sure I see. I’m just saying its a little dangerous to remove things from the common javascript files as we are not getting errors until runtime if things are broken. Maybe a grep on the code base would be nice here. Can’t think of any places it is used though.

I did that search through Netbeans on the core modules before deleting the function. Just requesting others, if I missed someplace

OK fine. But as far as I know there are other places with similar code (datamart, report table etc), is this working?

No. In datamart, I worked only for a minute and then realized, we didn’t use it in India and same with the dashboard, report tables. Will make those work in IE6 in the nextphase of fixes.

Well I believe most web-pages adhere to this, and I think it looks much better, yes…

Will make the padding symmetric then

Might be, but all javascripts in the system are written similar to the java code style, and it looks weird with different styles. If there are strong arguments for changing the style that is fine with me, but then everything should be changed.

I learnt the Java code styles from Java 1.2 … You can look here. and probably here. There will never be any strong argument for or against one’s style. It is philosophically what makes us human!! I just followed the norm and DHIS code base doesn’t seem to follow that

[Lars (Lars Øverland)]

I wouldn’t believe the necessity for IE6 if I had not seen it with my eyes… A training session in HP made me realize that it can be difficult for people to ask them to download Firefox and XP comes default with IE6. They won’t get updates without the internet and everywhere vanilla XP without any service packs… Thats what motivated me to get this fix done asap.

Yes but if we are able to deploy DHIS 2 we will definitely be able to deploy Firefox. Just bundle it with your CDs.

I tried JavaScript to get the png and some CSS hacks… but it made some parts slow and hence removed the transparency… and we aren’t really re-using most images (eg. different images in login.html and header)

Yes we are re-using the logo for most skins. I will rollback this if no one else do it.

Sure I see. I’m just saying its a little dangerous to remove things from the common javascript files as we are not getting errors until runtime if things are broken. Maybe a grep on the code base would be nice here. Can’t think of any places it is used though.

I did that search through Netbeans on the core modules before deleting the function. Just requesting others, if I missed someplace

OK fine. But as far as I know there are other places with similar code (datamart, report table etc), is this working?

No. In datamart, I worked only for a minute and then realized, we didn’t use it in India and same with the dashboard, report tables. Will make those work in IE6 in the nextphase of fixes.

You are not using datamart in India???

This is not a big case, but remember you are working on a global project. You benefit from efforts from the rest of the network. Then you should take some time to contribute back to the global system. Again, not a big case, but an important principle.

Well I believe most web-pages adhere to this, and I think it looks much better, yes…

Will make the padding symmetric then

Thank you.

Might be, but all javascripts in the system are written similar to the java code style, and it looks weird with different styles. If there are strong arguments for changing the style that is fine with me, but then everything should be changed.

I learnt the Java code styles from Java 1.2 … You can look here. and probably here. There will never be any strong argument for or against one’s style. It is philosophically what makes us human!! I just followed the norm and DHIS code base doesn’t seem to follow that

You are missing my point. Yes, Sun has a different code style than DHIS. I don’t have any strong preference in any direction. But things should look the same.