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

[Epic] Symfony Layout #32875

Closed
41 tasks done
jolelievre opened this issue Jun 11, 2023 · 1 comment
Closed
41 tasks done

[Epic] Symfony Layout #32875

jolelievre opened this issue Jun 11, 2023 · 1 comment
Labels
EPIC Macro-level issue gathering improvements & bugs related to a specific topic Symfony layout

Comments

@jolelievre
Copy link
Contributor

jolelievre commented Jun 11, 2023

The Symfony layout was initialized in this PR https://github.com/PrestaShop/PrestaShop/pull/32235/files it is meerly a POC that initializes this new feature, from this PR a lot of work is still left to do, some components must be added, some refacto is needed. This epic lists all the tasks eft to do for this feature to be full and for the Syfony layout to replace completely the legacy one.

Replacing legacy components

Create the Symfony/twig default layout

Replacing layout in legacy pages

Improvement about Context

Small refacto less impactful or less related

  • In LegacyControllerBridge (but in many other places, legacy and modern) the base URI depends on the shop from the context $this->legacyContext->getContext()->shop->getBaseURI() (but not only the URI depends on the shop), this can be problematic since it means if the DB is not set, not accessible or the Shop is not defined correctly the base URI of the whole BO is not accessible, and it also can not even be deduced from a CI perspective or for CLI commands without access to the database. We should think if this could be improved, maybe base this data on a configuration but that's a very simple suggestion that needs to be thought more deeply, and we should keep in mind the multishop problematic in this reflection.

Layout variables cleanup (probably not relevant anymore but kept here for now)

  • ControllerConfiguration will probably need to be refactored and cleaned For now, many legacy variables have been integrated for backward compatibility purposes without much reflection, but some are probably useless in a new twig theme. We should investigate which are still relevant, and which ones could be removed (keeping in mind that some modules may depend on them, any variable removed should have a legitimate alternative)
  • In the same spirit as the ControllerConfiguration that was introduced during the horizontal migration POC many variables are injected in the template's variables, some directly in SymfonyLayoutExtension that contains complete smart rendered templates used in the legacy layout (but maybe not useful anymore, or that could be refactored and rendered via twig components instead), and many more are built vie the Configurators that populate ControllerConfiguration::templateVars property, some investigation is needed about these variables to detect which were purely related to the legacy layout and can be removed, which should be replaced by more modern components/variables/alternatives, and which are really mandatory and need to be kept but maybe refactored
  • SymfonyLayoutExtension injects some blocks based on smarty, mostly modals, but it is also used to set some hard-coded smarty variables maybe it's not the right place, and maybe those modals should be removed completely. The extension should mostly be about setting global variables from the controller configuration. Although it's quite useful and easy to do it this way maybe it's not an ideal way (part of ControllerConfiguration refacto).
  • The @PrestaShop/Admin/Layout/default_layout.html.twig was mostly built based on admin-dev/themes/new-theme/template/layout.tpl the initial purpose is to keep the same markup as the previous to avoid unexpected side effects. But it may be interesting to refactor it later with better variables naming, maybe some pieces won't be relevant anymore. We should consider if the new variables are useful for legacy purposes (modules, overrides, ...) and if we need to keep the old names for backward compatibility. (this task is related to the first one about controller/layout variables)
  • Similarly in @PrestaShop/Admin/Layout/header.html.twig a lot of javascript variables are initialized, some of them may become useless.
  • The @PrestaShop/Admin/Layout/page_header_toolbar.html.twig can probably be optimized/simplified It seems like buttons are displayed twice for some reason
@hibatallahAouadni hibatallahAouadni added EPIC Macro-level issue gathering improvements & bugs related to a specific topic Symfony layout labels Jun 12, 2023
@matks
Copy link
Contributor

matks commented Aug 29, 2023

@MatShir MatShir closed this as completed Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPIC Macro-level issue gathering improvements & bugs related to a specific topic Symfony layout
Projects
None yet
Development

No branches or pull requests

4 participants