[drupal-devel] [task] Remove use of form_item() in taxonomy
Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. drumm -- View: http://drupal.org/node/17955 Edit: http://drupal.org/project/comments/add/17955
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Bèr Kessels Status: patch +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. Bèr Kessels Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 18:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too.
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Dries Status: patch All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) Dries Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 19:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 15:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense.
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Bèr Kessels Status: patch Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods. Bèr Kessels Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 18:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 14:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 17:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.)
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Dries Status: patch Ber: the confirmation page is less like the others we have. What makes you believe it looks more like the others? Dries Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 19:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 15:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 18:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) ------------------------------------------------------------------------ February 28, 2005 - 19:28 : Bèr Kessels Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods.
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Steven Status: patch Btw, I think the "confirmation API" that Bèr was talking about is just a simple form_ function to confirm various stuff in a consistent way. It's pretty simple to do. Steven Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 18:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 14:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 17:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) ------------------------------------------------------------------------ February 28, 2005 - 18:28 : Bèr Kessels Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods. ------------------------------------------------------------------------ March 1, 2005 - 21:16 : Dries Ber: the confirmation page is less like the others we have. What makes you believe it looks more like the others?
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Dries Status: patch This patch seems to break adding terms to vocabularies with no hierarchies. Dries Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 19:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 15:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 18:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) ------------------------------------------------------------------------ February 28, 2005 - 19:28 : Bèr Kessels Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods. ------------------------------------------------------------------------ March 1, 2005 - 22:16 : Dries Ber: the confirmation page is less like the others we have. What makes you believe it looks more like the others? ------------------------------------------------------------------------ March 1, 2005 - 22:22 : Steven Btw, I think the "confirmation API" that Bèr was talking about is just a simple form_ function to confirm various stuff in a consistent way. It's pretty simple to do.
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch For the confimation pages: it is consistent with the block and input format admin pages. For the adding terms, I am not suprised. Hopefully I can fix this in the next could days. drumm Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 11:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 07:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 10:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) ------------------------------------------------------------------------ February 28, 2005 - 11:28 : Bèr Kessels Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods. ------------------------------------------------------------------------ March 1, 2005 - 14:16 : Dries Ber: the confirmation page is less like the others we have. What makes you believe it looks more like the others? ------------------------------------------------------------------------ March 1, 2005 - 14:22 : Steven Btw, I think the "confirmation API" that Bèr was talking about is just a simple form_ function to confirm various stuff in a consistent way. It's pretty simple to do. ------------------------------------------------------------------------ March 1, 2005 - 15:26 : Dries This patch seems to break adding terms to vocabularies with no hierarchies.
Issue status update for http://drupal.org/node/17955 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/taxonomy.module_4.diff (5.1 KB) Fixes problem raised in #6 by Dries. drumm Previous comments: ------------------------------------------------------------------------ February 25, 2005 - 11:26 : drumm Attachment: http://drupal.org/files/issues/taxonomy.module_3.diff (4.94 KB) form_item() isn't right in a few cases. Such as confirmation pages and things which should be using other form functions. There was a subtle difference between form_select() and whatever taxonomy was doing. Thats why there is extra code here. I tested it a bit too. ------------------------------------------------------------------------ February 28, 2005 - 07:05 : Bèr Kessels +1 from me. Untill we have a general confirmation API, we should use something like this. Furthermore the form_select, instead of an ugly looped selector genreator makes sense. ------------------------------------------------------------------------ February 28, 2005 - 10:31 : Dries All other configuration pages (eg. locale module, menu module, etc.) all use form_item(). I'm likely to drop that bit, and have yet to see the feasability of a 'confirmation API' (it sounds difficult). (Ber: it is not clear from your review whether you tested the patch or not.) ------------------------------------------------------------------------ February 28, 2005 - 11:28 : Bèr Kessels Dries, I did not test the patch thouroughly. But from what I have seen and tested, the functionality does not change at all. Only the confirmation page looks more like the others we have. Off-topic: Some handbook page with a test-program might be handy. I, for example have very little professional experience with testing. i just install it and click around a bit, but there must be some good methods. ------------------------------------------------------------------------ March 1, 2005 - 14:16 : Dries Ber: the confirmation page is less like the others we have. What makes you believe it looks more like the others? ------------------------------------------------------------------------ March 1, 2005 - 14:22 : Steven Btw, I think the "confirmation API" that Bèr was talking about is just a simple form_ function to confirm various stuff in a consistent way. It's pretty simple to do. ------------------------------------------------------------------------ March 1, 2005 - 15:26 : Dries This patch seems to break adding terms to vocabularies with no hierarchies. ------------------------------------------------------------------------ March 1, 2005 - 20:42 : drumm For the confimation pages: it is consistent with the block and input format admin pages. For the adding terms, I am not suprised. Hopefully I can fix this in the next could days.
participants (4)
-
Bèr Kessels -
Dries
-
drumm
-
Steven