Issue #2152205 by joelpittet, mark.labrecque, Manuel Garcia, pakmanlh, steveoliver, hussainweb, shanethehat, jenlampton, kpa, AnythonyR, EVIIILJ, kgoel, Cottser, dsdeiz, hanpersand: Convert theme_date() to #theme input__date

Task

Convert theme_date() to #theme input__date.

Remaining tasks

  • Patch
  • Patch review
  • Manual testing
  • Profiling Not needed because it's not a template conversion - JP

Steps to test

@todo

Files: 
CommentFileSizeAuthor
#21 drupal-2152205-21.patch2.95 KBmark.labrecque
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,023 pass(es). View
#18 drupal-2152205-18.patch2.92 KBManuel Garcia
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,837 pass(es). View
#15 2152205-theme_date-15.patch2.92 KBpakmanlh
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View
#11 2152205-theme_date-11.patch2.91 KBjoelpittet
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,710 pass(es). View
#11 interdiff.txt835 bytesjoelpittet
#11 date-unpatched.txt55.26 KBjoelpittet
#11 date-patched.txt55.26 KBjoelpittet
#11 Kaleidoscope_–_date-patched_txt___date-unpatched_txt.png175.96 KBjoelpittet
#9 interdiff.txt753 bytesjoelpittet
#8 2152205-theme_date-7.patch2.86 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 64,168 pass(es). View
#8 interdiff.txt764 bytesjoelpittet
#5 2152205-theme_date-5.patch2.86 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 64,169 pass(es), 0 fail(s), and 284 exception(s). View
#5 interdiff.txt764 bytesjoelpittet
#3 2152205-theme_date-3.patch2.78 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 64,169 pass(es), 0 fail(s), and 284 exception(s). View
#3 interdiff.txt1.98 KBjoelpittet
#2 2152205-2-theme_date-input__date.patch2.23 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 59,438 pass(es). View

Comments

Cottser’s picture

Issue summary: View changes

Adding a commit message to the issue summary so the folks who already worked on #1898480: [meta] form.inc - Convert theme_ functions to Twig and in the Twig sandbox get credit.

joelpittet’s picture

Status: Active » Needs review
FileSize
2.23 KB
PASSED: [[SimpleTest]]: [MySQL] 59,438 pass(es). View

Split from form.inc twig conversion.

joelpittet’s picture

FileSize
1.98 KB
2.78 KB
FAILED: [[SimpleTest]]: [MySQL] 64,169 pass(es), 0 fail(s), and 284 exception(s). View

Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 3: 2152205-theme_date-3.patch, failed testing.

joelpittet’s picture

FileSize
764 bytes
2.86 KB
FAILED: [[SimpleTest]]: [MySQL] 64,169 pass(es), 0 fail(s), and 284 exception(s). View
+++ b/core/includes/form.inc
@@ -1117,29 +1117,30 @@ function password_confirm_validate($element, &$element_state) {
-  if (empty($element['attribute']['type'])) {
-    $element['attribute']['type'] = 'date';
-  }

This issue was a typo from core.

joelpittet’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2152205-theme_date-5.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
764 bytes
2.86 KB
PASSED: [[SimpleTest]]: [MySQL] 64,168 pass(es). View

And another typo... geez

joelpittet’s picture

FileSize
753 bytes

right interdiff...

joelpittet’s picture

Assigned: Unassigned » rfay
+++ b/core/includes/form.inc
@@ -1117,29 +1117,30 @@ function password_confirm_validate($element, &$element_state) {
-  if (empty($element['attribute']['type'])) {
-    $element['attribute']['type'] = 'date';
-  }
...
-  _form_set_attributes($element, array('form-' . $element['attribute']['type']));
+  _form_set_attributes($element, array('form-' . $element['#attributes']['type']));

FYI, this is a typo bug fix, that likely wouldn't have broken anything but could have since there is no 'attribute' key and it would always get set.

joelpittet’s picture

Assigned: rfay » joelpittet
Issue tags: -Needs manual testing
FileSize
175.96 KB
55.26 KB
55.26 KB
835 bytes
2.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,710 pass(es). View

Fixed an issue with the patch. Sorry @rfay, got excited with my new assignment abilities.

Also, here's a markup review, note the fix.

joelpittet’s picture

Assigned: joelpittet » Unassigned
Issue tags: -Needs profiling

Unassigned and likely doesn't need profiling as there is no twig template in this. Could use a review and RTBC;)

Cottser’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Tagging for reroll.

pakmanlh’s picture

Assigned: Unassigned » pakmanlh
pakmanlh’s picture

Assigned: pakmanlh » Unassigned
Status: Needs work » Needs review
FileSize
2.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View

Rerolled.

joelpittet’s picture

Issue tags: -Needs reroll

Prefect re-roll, thank you @pakmanlh!

Now just need someone to do a final review/RTBC

Manuel Garcia’s picture

Assigned: Unassigned » Manuel Garcia
Status: Needs review » Needs work
~/htdocs/drupal8$ git apply -v 2152205-theme_date-15.patch
Checking patch core/includes/form.inc...
error: while searching for:
}

/**
 * Returns HTML for an #date form element.
 *
 * Supports HTML5 types of 'date', 'datetime', 'datetime-local', and 'time'.
 * Falls back to a plain textfield. Used as a sub-element by the datetime
 * element type.
 *
 * @param array $variables
 *   An associative array containing:
 *   - element: An associative array containing the properties of the element.
 *     Properties used: #title, #value, #options, #description, #required,
 *     #attributes, #id, #name, #type, #min, #max, #step, #value, #size.
 *
 * @ingroup themeable
 */
function theme_date($variables) {
  $element = $variables['element'];
  if (empty($element['attribute']['type'])) {
    $element['attribute']['type'] = 'date';
  }
  element_set_attributes($element, array('id', 'name', 'type', 'min', 'max', 'step', 'value', 'size'));
  _form_set_attributes($element, array('form-' . $element['attribute']['type']));

  return '<input' . new Attribute($element['#attributes']) . ' />';
}

/**

error: patch failed: core/includes/form.inc:1194
error: core/includes/form.inc: patch does not apply
Checking patch core/includes/theme.inc...
Hunk #1 succeeded at 2727 (offset -152 lines).
Checking patch core/modules/system/system.module...
Hunk #1 succeeded at 505 (offset -15 lines).

Re-rolling...

Manuel Garcia’s picture

Assigned: Manuel Garcia » Unassigned
Status: Needs work » Needs review
FileSize
2.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,837 pass(es). View
Cottser’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Thanks @Manuel Garcia! Looks like this needs another reroll now that theme_checkboxes has been converted. Tagging for reroll.

mark.labrecque’s picture

Assigned: Unassigned » mark.labrecque
mark.labrecque’s picture

FileSize
2.95 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,023 pass(es). View
mark.labrecque’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
mark.labrecque’s picture

Assigned: mark.labrecque » Unassigned
joelpittet’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Reviewed #11, Don't need profiling because it's not a template.
Fixes a bug as demoed in screenshot in #11.
Removes a theme function.

Cottser’s picture

Issue summary: View changes

Patch looks good to me, updating suggested commit message.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.