From: Rafael C. <raf...@gm...> - 2014-09-13 18:35:54
|
Hi, A good practice, and a way to avoid problems with web browsers, is that tables MUST HAVE the same quantity of table-datacells ("<td>" tags). If is needed, you can use the "colspan" attribute ( quantity of columns a cell should span). In CustomerInquiry.php I fixed this problem. But, now I see the lsat version with the fix undo and again we have the same problems in some web browsers. Is there any reason for that? Regards, Rafael. |
From: ExsonQu <hex...@gm...> - 2014-09-14 11:20:22
|
Hi, Rafael, Sorry for make the revision not to discuss with you. The reason why I reverse it back is that I've found error notices that some parameters are fewer then required for the printf(). And at the mean time I think some new TD definitions make it hard to understand the business meaning of those codes although the code become short. I should have a discussion with the author who have revised before I revert it. I apologize for not having a discussion. Best regards! Exson -- View this message in context: http://weberp-accounting.1478800.n4.nabble.com/CustomerInquiry-php-W3C-html-code-tp4657643p4657646.html Sent from the web-ERP-developers mailing list archive at Nabble.com. |
From: Rafael C. <raf...@gm...> - 2014-09-15 19:26:46
|
Hi Exson, Please don't worry about this. I understand, I've also done it because he was in a hurry. What I am looking for is the reason for "des-commit". Specifically: If there is a bug or the code can be improved, I want to learn to prevent the recurrence of these errors; It is not fun to have people working on weekends or holidays for nothing. Best regards, Rafael. 2014-09-14 5:20 GMT-06:00 ExsonQu <hex...@gm...>: > Hi, Rafael, > > Sorry for make the revision not to discuss with you. > The reason why I reverse it back is that I've found error notices > that some parameters are fewer then required for the printf(). And at the > mean time I think some new TD definitions make it hard to understand the > business meaning of those codes although the code become short. > I should have a discussion with the author who have revised > before I revert it. > I apologize for not having a discussion. > Best regards! > Exson > > > > -- > View this message in context: > http://weberp-accounting.1478800.n4.nabble.com/CustomerInquiry-php-W3C-html-code-tp4657643p4657646.html > Sent from the web-ERP-developers mailing list archive at Nabble.com. > > > ------------------------------------------------------------------------------ > Want excitement? > Manually upgrade your production database. > When you want reliability, choose Perforce > Perforce version control. Predictably reliable. > > http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk > _______________________________________________ > Web-erp-developers mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/web-erp-developers > |
From: Rafael C. <raf...@gm...> - 2014-09-15 19:38:23
|
Hi All, *** ABOUT CustomerInquiry.php: *** 1. The problem: In some web-browser, incomplete table-rows ("<tr></tr>" tags with less quantity of columns) causes an unexpected behaviour (e.g. blanks and misaligned columns). E.g. Android 3.0.31/Navigator (Personal comment: I promote this ERPs as an ERP that can be run in a small device and can be run anywhere with a slow Internet connection, therefore it must run fine and fast). 2. The goal: fix this problem, document it, and have a code easy to understand for a newcomer (Personal comment: the last is the most difficult task. We can not "reset" our memory about the code to test if it is easy to understand for a newcomer). 3. What I did: I decomposed the code to understand it. After that, I reassembled. The result is one section with the constant part [format string and other components like html-links and called parameters (names) ]; that is BaseInfo, CreditInvoice, Allocation, PreviewInvoice, PreviewCredit and GLEntries section. And an other section with the variable part ("parameters"), where we can see the table-row structure. The main problem is still make evident the HTML table structure. 4. Secondary effects: 4.1. Regroups and comments the initial basic lines of script (includes/session.inc, $Title, $ViewTopic, $BookMark, includes/header.inc). Even without much impact, the purpose is to ensure that these lines are uniformly in all scripts that require it. 4.2. Deletes $tableheader variable. This variable is defined in line 253 and only used in next line (line 271). Nowhere else is used that variable. I think it's better to remove it to simplify the code and, theoretically(*), to become more fast the code. 4.3. Regroups some echo string functions. Instead of some consecutive echo language constructors, regroup them in only one instruction to simplify code. I think it makes the code easier to understand. 4.4. Adds the ascending class to Data, Type, Number and Reference columns. I found that some people look for an operation (line) seeking it by transaction date (over half of the time), type, number and reference (by customer's purchase order reference or by payment reference). This is a new feature to help users, and it is a lack if we compare CustomerInquiry.php with SupplierInquiry.php 4.5. $RootPath and $Theme in the "Constant Section". Those variables always hold the same values throughout the time this script runs. For clarity, I think it is better to separate those "constant variables" from the each row variables (printf parameters section). 5. Help wanted: 5.1. "Constant Section" variable names. Because those variables (BaseInfo..., CreditInvoice..., Allocation..., PreviewInvoice..., PreviewCredit... and GLEntries...) have more work than only format the string between the table-datacell tags ("<td></td>"), I "feel" that the suffix "FormatString" can conduct to an error. Comments? Suggestions? 5.2. "TD..." suffix. We need to show evident the quantity of columns in a table-row (pairs of "<td></td>" inside a "<tr></tr>"). I am not happy adding "_TD..." suffix, but I can not think of another idea. Comments? Suggestions? 5.3. Any suggestion to have a code easy to understand for a newcomer. --- (*) Insignificant gain: a fraction of absolute value function duration and some bytes. Best regards, Rafael. 2014-09-15 13:26 GMT-06:00 Rafael Chacón <raf...@gm...>: > Hi Exson, > > Please don't worry about this. I understand, I've also done it because he > was in a hurry. What I am looking for is the reason for "des-commit". > Specifically: If there is a bug or the code can be improved, I want to > learn to prevent the recurrence of these errors; It is not fun to have > people working on weekends or holidays for nothing. > > Best regards, Rafael. > > 2014-09-14 5:20 GMT-06:00 ExsonQu <hex...@gm...>: > > Hi, Rafael, >> >> Sorry for make the revision not to discuss with you. >> The reason why I reverse it back is that I've found error >> notices >> that some parameters are fewer then required for the printf(). And at the >> mean time I think some new TD definitions make it hard to understand the >> business meaning of those codes although the code become short. >> I should have a discussion with the author who have revised >> before I revert it. >> I apologize for not having a discussion. >> Best regards! >> Exson >> >> >> >> -- >> View this message in context: >> http://weberp-accounting.1478800.n4.nabble.com/CustomerInquiry-php-W3C-html-code-tp4657643p4657646.html >> Sent from the web-ERP-developers mailing list archive at Nabble.com. >> >> >> ------------------------------------------------------------------------------ >> Want excitement? >> Manually upgrade your production database. >> When you want reliability, choose Perforce >> Perforce version control. Predictably reliable. >> >> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk >> _______________________________________________ >> Web-erp-developers mailing list >> Web...@li... >> https://lists.sourceforge.net/lists/listinfo/web-erp-developers >> > > |
From: Phil D. <ph...@lo...> - 2014-09-16 09:38:34
|
Hola Rafael, As we have corresponded off list, I know that these comments within will be no surprise to you - but for the benefit of anyone else. On 16/09/14 07:31, Rafael Chacón wrote: > Hi All, > > *** ABOUT CustomerInquiry.php: *** > > 1. The problem: In some web-browser, incomplete table-rows > ("<tr></tr>" tags with less quantity of columns) causes an unexpected > behaviour (e.g. blanks and misaligned columns). E.g. Android > 3.0.31/Navigator (Personal comment: I promote this ERPs as an ERP that > can be run in a small device and can be run anywhere with a slow > Internet connection, therefore it must run fine and fast). > Well I am not sure all the screens are so good on smaller devices. > 2. The goal: fix this problem, document it, and have a code easy to > understand for a newcomer (Personal comment: the last is the most > difficult task. We can not "reset" our memory about the code to test > if it is easy to understand for a newcomer). Yes it is subjective - but consistency with existing code and strict adherence to the conventions/style as per http://www.weberp.org/CodingConventions.html should cover it. > > 3. What I did: I decomposed the code to understand it. After that, I > reassembled. The result is one section with the constant part [format > string and other components like html-links and called parameters > (names) ]; that is BaseInfo, CreditInvoice, Allocation, > PreviewInvoice, PreviewCredit and GLEntries section. And an other > section with the variable part ("parameters"), where we can see the > table-row structure. The main problem is still make evident the HTML > table structure. > The problem with that though is that is that it ended up with a LOT of changed code and the actual fix was obscured. Much better I think to evolve with just the code necessary to effect the bug fix > 4. Secondary effects: > > 4.1. Regroups and comments the initial basic lines of script > (includes/session.inc, $Title, $ViewTopic, $BookMark, > includes/header.inc). Even without much impact, the purpose is to > ensure that these lines are uniformly in all scripts that require it. > All great contributions these. > 4.2. Deletes $tableheader variable. This variable is defined in line > 253 and only used in next line (line 271). Nowhere else is used that > variable. I think it's better to remove it to simplify the code and, > theoretically(*), to become more fast the code. > Yep - if used several times it messes with the table sorting javascript anyway. > 4.3. Regroups some echo string functions. Instead of some consecutive > echo language constructors, regroup them in only one instruction to > simplify code. I think it makes the code easier to understand. > Me to - probably reduces the work of the parser too. > 4.4. Adds the ascending class to Data, Type, Number and Reference > columns. I found that some people look for an operation (line) seeking > it by transaction date (over half of the time), type, number and > reference (by customer's purchase order reference or by payment > reference). This is a new feature to help users, and it is a lack if > we compare CustomerInquiry.php with SupplierInquiry.php Yep worthwhile for most scripts > > 4.5. $RootPath and $Theme in the "Constant Section". Those variables > always hold the same values throughout the time this script runs. > For clarity, I think it is better to separate those "constant > variables" from the each row variables (printf parameters section). > Not quite sure what you mean here? > 5. Help wanted: > > 5.1. "Constant Section" variable names. Because those variables > (BaseInfo..., CreditInvoice..., Allocation..., PreviewInvoice..., > PreviewCredit... and GLEntries...) have more work than only format the > string between the table-datacell tags ("<td></td>"), I "feel" that > the suffix "FormatString" can conduct to an error. Comments? Suggestions? > Can't these variables be made to have the correct number of cells ("<td></td>") ? My impression is that this is all that is required to fix this script? > 5.2. "TD..." suffix. We need to show evident the quantity of columns > in a table-row (pairs of "<td></td>" inside a "<tr></tr>"). I am not > happy adding "_TD..." suffix, but I can not think of another idea. > Comments? Suggestions? I wasn't keen on the TD suffixes and initially didn't get what they were. Better off just to make the variables have the correct number of cells. > > 5.3. Any suggestion to have a code easy to understand for a newcomer. > Follow http://www.weberp.org/CodingConventions.html rigorously. Of course this is just my take on this and the way much of the code is laid out as far as possible. If some of it isn't then changes to bring it in line are most welcome. Every so often someone comes up with a bright idea to change the way the code is. I am not interested re-writing the code a different way. I am interested in bug fixes and new functionality. However, I took Andrew's point that abstracting some code to functions would make some changes easier and use less characters and there is a middle ground with abstraction. I think we have a reasonable compromise - and I make no apology for erring on the side of readability - it is a core goal. Phil |
From: Phil D. <ph...@lo...> - 2014-09-19 07:21:21
|
Thanks Tim, Looks good to me Not sure why you haven't committed this yourself though? Let me know if you want me to do it for you? We've had this discussion before and it gets boring for everyone .... you still have access to the code. On the abstraction issue... yes some abstraction makes sense and we have plenty already. I am not into making a function for every little bit of html we echo though as I feel the presentation code adds context. I don't wish to debate this ... again! A common customer search function though might be a useful in SQL_CommonFunctions.inc Phil Phil Daintree Logic Works Ltd - +64 (0)275 567890 http://www.logicworks.co.nz On 19/09/14 04:13, Tim Schofield wrote: > While waiting for Phil's ruling on this I have redone > CustomerInquiry.php in what I think is a more readable way, that > always has the same number of columns no matter what the transaction, > and changed the original code to comply with the guidelines. > > Am attaching the new script, > Tim > > On 17 September 2014 12:15, Tim Schofield <tim...@gm...> wrote: >> Hi Phil, still no polite or useful reply to this. I think it is >> important for us developers to have a ruling on what can and what >> cannot be abstracted. >> >> Yours in anticipation, >> Tim >> >> On 16 September 2014 11:05, Tim Schofield <tim...@gm...> wrote: >>> Phil, >>> >>> Do you stand by your comments that the stock search should be >>> abstracted to a function in SQLCommonFunctions.inc? >>> >>> Thanks >>> Tim >>> >>> On 16 September 2014 09:53, Phil Daintree <ph...@lo...> wrote: >>>> Hola Rafael, >>>> >>>> As we have corresponded off list, I know that these comments within will >>>> be no surprise to you - but for the benefit of anyone else. >>>> >>>> >>>> On 16/09/14 07:31, Rafael Chacón wrote: >>>>> Hi All, >>>>> >>>>> *** ABOUT CustomerInquiry.php: *** >>>>> >>>>> 1. The problem: In some web-browser, incomplete table-rows >>>>> ("<tr></tr>" tags with less quantity of columns) causes an unexpected >>>>> behaviour (e.g. blanks and misaligned columns). E.g. Android >>>>> 3.0.31/Navigator (Personal comment: I promote this ERPs as an ERP that >>>>> can be run in a small device and can be run anywhere with a slow >>>>> Internet connection, therefore it must run fine and fast). >>>>> >>>> Well I am not sure all the screens are so good on smaller devices. >>>> >>>> >>>>> 2. The goal: fix this problem, document it, and have a code easy to >>>>> understand for a newcomer (Personal comment: the last is the most >>>>> difficult task. We can not "reset" our memory about the code to test >>>>> if it is easy to understand for a newcomer). >>>> >>>> Yes it is subjective - but consistency with existing code and strict >>>> adherence to the conventions/style as per >>>> >>>> http://www.weberp.org/CodingConventions.html >>>> >>>> should cover it. >>>> >>>>> 3. What I did: I decomposed the code to understand it. After that, I >>>>> reassembled. The result is one section with the constant part [format >>>>> string and other components like html-links and called parameters >>>>> (names) ]; that is BaseInfo, CreditInvoice, Allocation, >>>>> PreviewInvoice, PreviewCredit and GLEntries section. And an other >>>>> section with the variable part ("parameters"), where we can see the >>>>> table-row structure. The main problem is still make evident the HTML >>>>> table structure. >>>>> >>>> The problem with that though is that is that it ended up with a LOT of >>>> changed code and the actual fix was obscured. Much better I think to >>>> evolve with just the code necessary to effect the bug fix >>>> >>>> >>>>> 4. Secondary effects: >>>>> >>>>> 4.1. Regroups and comments the initial basic lines of script >>>>> (includes/session.inc, $Title, $ViewTopic, $BookMark, >>>>> includes/header.inc). Even without much impact, the purpose is to >>>>> ensure that these lines are uniformly in all scripts that require it. >>>>> >>>> All great contributions these. >>>> >>>> >>>>> 4.2. Deletes $tableheader variable. This variable is defined in line >>>>> 253 and only used in next line (line 271). Nowhere else is used that >>>>> variable. I think it's better to remove it to simplify the code and, >>>>> theoretically(*), to become more fast the code. >>>>> >>>> Yep - if used several times it messes with the table sorting javascript >>>> anyway. >>>> >>>>> 4.3. Regroups some echo string functions. Instead of some consecutive >>>>> echo language constructors, regroup them in only one instruction to >>>>> simplify code. I think it makes the code easier to understand. >>>>> >>>> Me too - probably reduces the work of the parser too. >>>> >>>>> 4.4. Adds the ascending class to Data, Type, Number and Reference >>>>> columns. I found that some people look for an operation (line) seeking >>>>> it by transaction date (over half of the time), type, number and >>>>> reference (by customer's purchase order reference or by payment >>>>> reference). This is a new feature to help users, and it is a lack if >>>>> we compare CustomerInquiry.php with SupplierInquiry.php >>>> Yep worthwhile for most scripts >>>>> 4.5. $RootPath and $Theme in the "Constant Section". Those variables >>>>> always hold the same values throughout the time this script runs. >>>>> For clarity, I think it is better to separate those "constant >>>>> variables" from the each row variables (printf parameters section). >>>>> >>>> Not quite sure what you mean here? >>>> >>>>> 5. Help wanted: >>>>> >>>>> 5.1. "Constant Section" variable names. Because those variables >>>>> (BaseInfo..., CreditInvoice..., Allocation..., PreviewInvoice..., >>>>> PreviewCredit... and GLEntries...) have more work than only format the >>>>> string between the table-datacell tags ("<td></td>"), I "feel" that >>>>> the suffix "FormatString" can conduct to an error. Comments? Suggestions? >>>>> >>>> Can't these variables be made to have the correct number of cells >>>> ("<td></td>") ? My impression is that this is all that is required to >>>> fix this script? >>>> >>>>> 5.2. "TD..." suffix. We need to show evident the quantity of columns >>>>> in a table-row (pairs of "<td></td>" inside a "<tr></tr>"). I am not >>>>> happy adding "_TD..." suffix, but I can not think of another idea. >>>>> Comments? Suggestions? >>>> I wasn't keen on the TD suffixes and initially didn't get what they >>>> were. Better off just to make the variables have the correct number of >>>> cells. >>>>> 5.3. Any suggestion to have a code easy to understand for a newcomer. >>>>> >>>> Follow >>>> >>>> http://www.weberp.org/CodingConventions.html >>>> >>>> rigorously. Of course this is just my take on this and the way much of >>>> the code is laid out as far as possible. If some of it isn't then >>>> changes to bring it in line are most welcome. >>>> >>>> Every so often someone comes up with a bright idea to change the way the >>>> code is. >>>> >>>> I am not interested re-writing the code a different way. >>>> >>>> I am interested in bug fixes and new functionality. >>>> >>>> However, I took Andrew's point that abstracting some code to functions >>>> would make some changes easier and use less characters and there is a >>>> middle ground with abstraction. I think we have a reasonable compromise >>>> - and I make no apology for erring on the side of readability - it is a >>>> core goal. >>>> >>>> Phil |