Create a form that allows to change the path used for the frontpage

I translated the initial post to English. It is not clear to me what is meant with "a preview link" and "implement this without overlay".

I just had a quick look, some remarks:

Naming of form builders

I would rename the functions that provide the form builders. The current function names paddle_get_front_page() and paddle_set_front_page() give the impression that these functions can be used to respectively retrieve or set the current front page path. Rename them to e.g. paddle_frontpage_form().

Purpose of 'View current frontpage' page

I cannot imagine what the purpose of this page is. This is not defined in the scope of this issue. This data is duplicated on the 'set front page' page, and if people want to have a link to the front page, they can just as well click on the "Home" button in the main menu or click on the logo.

Do not use the form builder for generic page callbacks

paddle_get_front_page() is using a form builder and a call to drupal_get_form() for rendering what is essentially a link. This can be implemented by using a regular page callback and returning a render array.

Purpose of 'View current frontpage' page

If you look at the latest wireframes we received, there are separate pages for just viewing and editing. If you want a debate about that, the right person to talk to is the product manager ;) You are right that the page for just viewing the current setting originally was out of scope of this issue, but it's a small one to add so can be included in this issue IMHO.

Alright, thanks I had not looked at the wireframes. I can imagine though that this will be removed in a subsequent revision of the wireframes :)

Reviewing.

Proper review this time :)

I have unfortunately been unable to test the functionality itself. Especially the autocompleting looks very promising. There has been good progress on this but there are a number of things that need to be addressed first before I can do a full functional test.

Move hooks out of the .profile file into a separate module

The functionality is implemented in the .profile file. This is not the right place, this file should only contain stuff that is needed during the installation process. The code should be put in a separate module. Also rename the related files such as paddle.profile.css and paddle.profile.js.

The functionality was not working on my install because of this, after this is fixed this should be tested again.

Remove return value documentation from hook implementations and form callbacks

Hook implementations, form builders and page callbacks use a special short documentation format. It is not needed to document return values. Hook implementations also do not need to have their parameters documented. Refer to API documentation and comment standards, search for "Hook implementation", "Form-generating functions", "hook_menu() page router callback functions".

An example:

+/**
+ * Implements hook_menu().
+ * @return array
+ */
+function paddle_menu() {

Move functionality under 'Configuration' rather than 'Structure'

function paddle_menu() {
...
  $items['admin/structure/frontpage/edit'] = array(
...
  );
}

This is a minor configuration setting which will be rarely changed. It has no place under Structure, and certainly not as a root item. Best thing at this point would be to create a new 'Paddle' section in 'admin/config" and then place this under 'admin/config/paddle/frontpage'.

Example of defining a section under admin/config (from system_menu()):

  $items['admin/config/services'] = array(
    'title' => 'Web services',
    'description' => 'Tools related to web services.',
    'position' => 'right',
    'weight' => 0,
    'page callback' => 'system_admin_menu_block_page',
    'access arguments' => array('access administration pages'),
    'file' => 'system.admin.inc',
  );

Use local actions

The two menu items "view" and "edit" are now defined as NORMAL_MENU_ITEM. This will make them appear in dropdown menus etc. It would be better to define them as MENU_LOCAL_TASK, this way they will appear as tabs, and they will be picked up by the "contextual toolbar" module that is in development.

You will need:

1. admin/config/paddle/frontpage: NORMAL_MENU_ITEM. This will be used to anchor the local tasks to.
2. admin/config/paddle/frontpage/view: MENU_DEFAULT_LOCAL_TASK. The "view" task is the default, this will be highlighted and shown initially when the user visits this page.
3. admin/config/paddle/frontpage/edit: MENU_LOCAL_TASK. The "edit" task is accessed by clicking on a tab (+ will be in the contextual toolbar in the future).

There is well-documented example of this in the Examples for Developers module: Local tasks example.

Update path

function paddle_menu() {
...
  $items['menupaths/autocomplete/%'] = array(

This path has probably been copied from an example and needs to be updated to admin/config/paddle/frontpage/autocomplete/%'.

Indentation of chained functions

Use a standard indentation of 2 spaces when chaining functions. For example here

function _paddle_get_matches_for_nodes($title = '') {
...
  $result = $query
          ->fields('n')
          ->limit(variable_get('paddle_auto_max_items', 20))
          ->execute();

This is not very well documented in the coding standards, but this is the way it is done in core. An example from the node module:

function node_user_cancel($edit, $account, $method) {
...
  $nodes = db_select('node', 'n')
    ->fields('n', array('nid'))
    ->condition('uid', $account->uid)
    ->execute()
    ->fetchCol();
...

Don't use the !important directive

#autocomplete {
  top: 100px !important;
  left: 16px !important;
  overflow-y: scroll;
  max-height: 300px;
}

Never use the !important directive in CSS, many developers have been subjected to savage violence by hordes of raging frontenders over this. This can usually be fixed by using a more specific selector.

Test incomplete

The test does not seem to be complete, there is some commented code left in here.

Coder review warnings

Coder review throws a number of warnings. To run these tests, install the Coder module and run drush dcs [filename] on your files. I have only checked the .profile file, but all files should be checked:

$ drush dl coder
$ drush en coder -y
$ drush dcs paddle.profile

FILE: /home/pieter/v/paddle/paddle/profiles/paddle/paddle.profile
--------------------------------------------------------------------------------
FOUND 26 ERROR(S) AND 3 WARNING(S) AFFECTING 23 LINE(S)
--------------------------------------------------------------------------------
  10 | WARNING | Hook implementations should not duplicate @return
     |         | documentation
  10 | ERROR   | Missing comment for @return statement
  13 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  23 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  32 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  48 | ERROR   | Missing comment for @return statement
  52 | ERROR   | Whitespace found at end of line
  55 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  63 | WARNING | Line exceeds 80 characters; contains 91 characters
  63 | ERROR   | Function comment short description must be on a single line
  64 | WARNING | Line exceeds 80 characters; contains 122 characters
  66 | ERROR   | Missing comment for @return statement
  87 | ERROR   | Array indentation error, expected 6 spaces but found 8
  88 | ERROR   | Array indentation error, expected 4 spaces but found 6
  88 | ERROR   | Array closing indentation error, expected 4 spaces but found 6
 100 | ERROR   | There must be an empty line before the parameter block
 100 | ERROR   | Last parameter comment requires a blank newline after it
 100 | ERROR   | Expected 1 space before variable type
 100 | ERROR   | Missing comment for param "$string" at position 1
 101 | ERROR   | Missing comment for @return statement
 110 | ERROR   | Whitespace found at end of line
 118 | ERROR   | Whitespace found at end of line
 139 | ERROR   | Missing parameter type at position 1
 147 | ERROR   | Whitespace found at end of line
 164 | ERROR   | Function comment short description must be on a single line
 167 | ERROR   | Missing parameter type at position 1
 188 | ERROR   | Function comment short description must be on a single line
 191 | ERROR   | Missing parameter type at position 1
 210 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------

Move functionality under 'Configuration' rather than 'Structure'

I think we need to focus on the targets we have, on the wireframes this is clearly located under Structure. So for now it needs to stay there.

Just a remark: the reason I was unable to test the functionality in #9 was because I stupidly installed the site with drush si without specifying the right profile to use.

@Cyberwolf: placing this under "Structure" does not meet the D7 user experience guidelines (ref http://www.d7ux.org/), so I would suggest to start a discussion about improving the wireframes.

FILE: paddle/paddle.profile
--------------------------------------------------------------------------------
FOUND 22 ERROR(S) AND 2 WARNING(S) AFFECTING 19 LINE(S)
--------------------------------------------------------------------------------
  12 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  24 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  34 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  43 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  59 | ERROR   | Missing comment for @return statement
  66 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  74 | WARNING | Line exceeds 80 characters; contains 91 characters
  74 | ERROR   | Function comment short description must be on a single line
  75 | WARNING | Line exceeds 80 characters; contains 122 characters
  77 | ERROR   | Missing comment for @return statement
  98 | ERROR   | Array indentation error, expected 6 spaces but found 8
  99 | ERROR   | Array indentation error, expected 4 spaces but found 6
  99 | ERROR   | Array closing indentation error, expected 4 spaces but found 6
 111 | ERROR   | There must be an empty line before the parameter block
 111 | ERROR   | Last parameter comment requires a blank newline after it
 111 | ERROR   | Expected 1 space before variable type
 111 | ERROR   | Missing comment for param "$string" at position 1
 112 | ERROR   | Missing comment for @return statement
 150 | ERROR   | Missing parameter type at position 1
 175 | ERROR   | Function comment short description must be on a single line
 178 | ERROR   | Missing parameter type at position 1
 199 | ERROR   | Function comment short description must be on a single line
 202 | ERROR   | Missing parameter type at position 1
 221 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: paddle/tests/paddle.test
--------------------------------------------------------------------------------
FOUND 7 ERROR(S) AFFECTING 7 LINE(S)
--------------------------------------------------------------------------------
 58 | ERROR | Function comment short description must end with a full stop
 61 | ERROR | Inline comments must end in full-stops, exclamation marks, or
    |       | question marks
 65 | ERROR | Inline comments must end in full-stops, exclamation marks, or
    |       | question marks
 69 | ERROR | Inline doc block comments are not allowed; use "// Comment"
    |       | instead
 79 | ERROR | Inline doc block comments are not allowed; use "// Comment"
    |       | instead
 85 | ERROR | Inline comments must end in full-stops, exclamation marks, or
    |       | question marks
 88 | ERROR | Inline comments must end in full-stops, exclamation marks, or
    |       | question marks
--------------------------------------------------------------------------------

Use Variable Translation to set Frontpage for each individual language.

Translated to English.