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

Comments

star-szr’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
StatusFileSize
new2.23 KB

Split from form.inc twig conversion.

joelpittet’s picture

StatusFileSize
new1.98 KB
new2.78 KB

Re-rolled.

Status: Needs review » Needs work

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

joelpittet’s picture

StatusFileSize
new764 bytes
new2.86 KB
+++ 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
StatusFileSize
new764 bytes
new2.86 KB

And another typo... geez

joelpittet’s picture

StatusFileSize
new753 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
StatusFileSize
new175.96 KB
new55.26 KB
new55.26 KB
new835 bytes
new2.91 KB

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;)

star-szr’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
StatusFileSize
new2.92 KB

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
StatusFileSize
new2.92 KB
star-szr’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

StatusFileSize
new2.95 KB
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.

star-szr’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.