Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich text transformation improvement #29463

Open
6 tasks done
Hlavtox opened this issue Aug 28, 2022 · 3 comments
Open
6 tasks done

Rich text transformation improvement #29463

Hlavtox opened this issue Aug 28, 2022 · 3 comments
Labels
CO Category: Core Developer Feature Developer-oriented feature Feature Type: New Feature Needs Specs Status: issue needs to be specified

Comments

@Hlavtox
Copy link
Contributor

Hlavtox commented Aug 28, 2022

Prerequisites

Is your feature request related to a problem?

A developer feature.

In front ProductController, there is following code you can see below. It deals with transforming a description that comes from backoffice. It alters images a bit in product descriptions and sets some classes for responsivity to them.

I think:

  • It's not very good place for it
  • It's not reusable.
  • It's not used for other descriptions.
    protected function transformDescriptionWithImg($desc)
    {
        $reg = '/\[img\-([0-9]+)\-(left|right)\-([a-zA-Z0-9-_]+)\]/';
        while (preg_match($reg, $desc, $matches)) {
            $link_lmg = $this->context->link->getImageLink($this->product->link_rewrite, $this->product->id . '-' . $matches[1], $matches[3]);
            $class = $matches[2] == 'left' ? 'class="imageFloatLeft"' : 'class="imageFloatRight"';
            $html_img = '<img src="' . $link_lmg . '" alt="" ' . $class . '/>';
            $desc = str_replace($matches[0], $html_img, $desc);
        }

        return $desc;
    }

Describe the solution you'd like

  • There is a filterHtmlContent hook in ObjectPresenter that deals with transforming all HTML fields. This is a hook where theme modules can hook into and do their job - transform the HTML coming from BO to FO in whatever way they like.
  • I think this code should not be in a core, as it's a theme specific code. Where to put it?

Alternatives you've considered

No response

Additional context

Ping @PrestaShop/prestashop-maintainers for specifications.

Do you plan to work on this feature?

  • I'm willing to contribute a formal specification.
  • I'm willing to provide any wireframes or design assets required for this feature.
  • I'm willing to submit a Pull Request that implements this feature.
  • I'm willing to help verify that the implemented feature works as intended and produces no unintended side effects.
@Hlavtox Hlavtox added Feature Type: New Feature New New issue not yet processed by QA labels Aug 28, 2022
@Hlavtox Hlavtox changed the title Rich text transformation Rich text transformation improvement Aug 28, 2022
@yanmakouf
Copy link
Contributor

@Hlavtox thanks for your proposition/request. I think it is worth investigating, and, in case, improve. There are so much things that need to be improved, we will look into it and see if it is doeable or not according to the release plan, keep in mind that we cannot introduce bc breaks in a minor version so switching it would imho ( i'm new) be in the next major (provided it is approved), since moving it would probably induce broken usage we didn't think of.

@Hlavtox
Copy link
Contributor Author

Hlavtox commented Aug 28, 2022

@yanmakouf Yes Yani, no BCs. The question is, what to do with this code? I think this code should be in a theme related module I think. Or just removed from the default theme, because it can be styled with CSS.

@sLorenzini
Copy link
Contributor

Hello @Hlavtox,

Thank you for your suggestion. The Maintainer Team will take it into consideration for future developments.
Please be aware that there is no guarantee that this feature will be developed anytime soon.
We’re waiting for your PR 🚀

Thank you

@sLorenzini sLorenzini added CO Category: Core Needs Specs Status: issue needs to be specified Developer Feature Developer-oriented feature and removed New New issue not yet processed by QA labels Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CO Category: Core Developer Feature Developer-oriented feature Feature Type: New Feature Needs Specs Status: issue needs to be specified
Projects
None yet
Development

No branches or pull requests

3 participants