processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

InputfieldRepeater’s $colorPrefix gets broken because of $page->getText() #1732

Open romaincazier opened 1 year ago

romaincazier commented 1 year ago

Short description of the issue

When defining both a color and fields in the repeater label the $colorPrefix get broken when it goes through $page->getText(). The \r get removed, breaking the last str_replace. This is also an issue for InputfieldRepeaterMatrix.

Suggestion for a possible fix

Use something other than \r while remaining foolproof, something like "pwCOLORpw".

Setup/Environment

romaincazier commented 1 year ago

Another issue/suggestion related to this: in a nested repeater setup, this feature doesn’t work because of how the style is added to the admin theme and thus is never rendered. A simple fix could be to use $this->prependMarkup to add the <style> tag instead:

if(!$isPost && count($typeStyles)) {
    $this->prependMarkup .= "<style type='text/css'>" . implode("\n", $typeStyles) . "</style>";
    // $this->wire()->adminTheme->addExtraMarkup('head', $styles);
}

In my testing this works and I don’t think there is any downside doing so as the css is very “local”.

ryancramerdesign commented 1 year ago

Thanks @romaincazier I've pushed your suggested fixes for these.

romaincazier commented 1 year ago

Thanks!

romaincazier commented 1 year ago

I'm reopening this issue because the added "\t" in the color prefix gets cleared later on by a trim() call resulting in this condition being falsy and thus never applying the color.

(actually it's working if I put the hex code at the end of label instead of at the beginning. I tested it after seeing this mentioned in RepeaterMatrix’s notes)

Toutouwai commented 1 year ago

@ryancramerdesign, before Repeater Matrix included the header colour feature I added a similar feature in a third-party module. I did this via a separate type=color input in the matrix type config. Maybe it would be more robust and user-friendly if Repeater Matrix took a similar approach?

romaincazier commented 4 months ago

@ryancramerdesign I’m using this issue to add a feature request:

I was looking for a way to add a specific color to a specific repeater item based on some value (in a hook) but couldn’t do so as the style gets applied to all repeater items (because $itemTypeName is empty).

Trying things I ended with these edits in the core:

You just need to rename $typeStyles to $styles (and replace subsequent occurrences), and then replace InputfieldRepeater L503-506 with:

$styles[] = 
    ".Inputfield_$this->name .InputfieldContent " . 
    ".InputfieldRepeaterItem[data-page=\"$page->id\"] > .InputfieldHeader " . 
    "{ background-color: #$matches[1]; outline-color: #$matches[1] }";

Since there’s no check whether the style for a specific type has already been added or not I feel this is ok. If needed we could always first check for a valid $itemTypeName and subsequently use this instead of $page->id.

Coming back to this issue I also liked @Toutouwai’s idea but it would unfortunately be a breaking change.