The quality of the code is not good in some places:
<?php
t("You need to !link first before being able to set them here.", array('!link' => l("create contact form categories", "admin/build/contact/add")));
>?
Sentences should never be broken up. Translating such strings is a menace as the context is gone. In some cases it can also cause completely broken translations.

Also, why is $_SESSION being used to store the contact form category? The regular contact form uses an element for this. Why not change the element to #type == 'value', so the regular form submit handler can deal with it?

Changing the site-wide contact form has nothing to do with contact forms in blocks.

Even though every category has its own block, there is no single block that has all categories, which would be very useful in combination with a module like Panels.

Comments

fuerst’s picture

fuerst commented
Status: Active » Closed (won't fix)
  • Sentences should never be broken up: I rather like the idea of having user friendly output than satisfying programmers needs. I'm not a native english speaker though - you may find a better way to express things which fits both needs.
  • why is $_SESSION being used to store the contact form category? The regular contact form uses an element for this.: I'm used to use $_SESSION for values used between page requests. Using an form element is also good.
  • Changing the site-wide contact form has nothing to do with contact forms in blocks.: Don't habe a glue what you mean.
  • Even though every category has its own block, there is no single block that has all categories...: Would be nice to have.

Feel free to change what you don't like and provide patches. Would be nice if you would split your topics into different issues. Since I would not work on this I close the issue. Feel free to reopen when more discussion is needed.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Closed (won't fix) » Active

The thing with breaking up sentences is not that you create user friendly output. You hinder translators, which can cause translations and therefore output to be not user friendly.

This module provides options to change the site-wide contact form. This has nothing to do with the module's purpose, which is to expose the contact form or its different categories as blocks.

$_SESSION is not necessarily bad. It is, however, if you use it in this way. Because of the use of $_SESSION extra code needs to be written to process it. If you add a form element, just like the standard contact form does, you can also use that form's submit handler. It also prevents bugs with multiple contact forms being displayed (edge case).

My point is that ideally you should sit down and look at what parts of the code are not according to the Drupal (coding) standards and whether your approach really is for the better. I need contact forms as blocks for Drupal 7 (but a slightly different way) and I think it's a waste to create a completely new project for that, but I cannot simply step in here and make major changes without your consent *and* your attention, since the issue looks like is hasn't been given any attention in a long time.

fuerst’s picture

fuerst commented
  • options to change the site-wide contact form: I think that option should be available. At least I do need to disable contact categories from the site-wide contact form as soon as the categorie is used as a block. May be that setting should be placed at the particular Contact category edit page like admin/build/contact/edit/1 instead of having it's own settings page (admin/settings/contact_form_blocks)
  • breaking up sentences: I understand your concern about this but also want to have explaining messages for the user. I will keep an eye at easy translatable strings though. Anyway, the sentence you refering to above is the only broken one and it will not be necessary anymore if the options to change the site-wide contact form will be placed in the Contact category edit page as described above.
  • $_SESSION is gone through the fix from #687706, see http://drupal.org/cvs?commit=498326