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

SE-33 - Review and fix the luxe-jahia-demo #126

Closed
jahia-ci opened this issue Feb 21, 2024 · 3 comments
Closed

SE-33 - Review and fix the luxe-jahia-demo #126

jahia-ci opened this issue Feb 21, 2024 · 3 comments
Assignees
Labels
Jira:SE Jira project this issue was imported from Priority:Unspecified Priority of the ticket Scope-change Task team-goat

Comments

@jahia-ci
Copy link
Contributor

jahia-ci commented Feb 21, 2024

Imported from Jira, on Thu Jan 30, 2025
Issue: SE-33 in project: Sales Engineering
Priority: Unspecified Type: Task
Reporter: @Fgerthoffert (Francois Gerthoffert)
Assignee: @Janin-Michel-Mathias (Janin Michel-Mathias)
Created: Wed Feb 21, 2024, closed: Fri Mar 8, 2024
Status: Closed (resolution: Done)
Parent Epic: BACKLOG-22266 - luxe demo implementation P1 [JIRA] (Closed)

The goal of this ticket is to review work done for https://github.com/Jahia/luxe-jahia-demo, and then:

  • highlight in this ticket important findings (i.e. when further discussions are needed),
  • Fix/improve implementation whereever necessary

Do not start a major refactor. If changes are required in other codebases (such as npm-modules-engines), create the corresponding tickets. The goal of this ticket is not to fix/provide changes for npm-modules-engine.

Tickets that triggered the review:

  • BACKLOG-22267
  • BACKLOG-22268
  • BACKLOG-22269
  • BACKLOG-22271
  • BACKLOG-22273
  • BACKLOG-22274
@jahia-ci
Copy link
Contributor Author

Comment by: @jahia-ci (Serge Huber [X]) on Fri Feb 23, 2024 edited

I have renamed all the components / views / templates to following the latest NPM structure guidelines, making it much easier to understand the relationship between nodetypes and files.

Here are the open points that need to be addressed:

  • The creation of ids for the components is really tedious, we really need to implement : https://jira.jahia.org/browse/BACKLOG-22423
  • The MainLayout component seems to be waiting on nodetypes restrictions and limit support for absolute areas. This should now work and should be activated using JAbsoluteArea and not JRender.
  • All the Webpack configuration were regrouped under a single files, but we might want to review the ordering of the configurations and how changes deploy (including when watches are active).
  • There are some problems integrating client-side Javascript for the back button, maybe we will need to integrate the work that has been done on React hydration in this project ? (see https://jira.jahia.org/browse/BACKLOG-22335)
  • There is a tests directory in the project but there are no Cypress tests in there. What is the status of this part of the project ?
  • The project uses SASS/SCSS for CSS styling. As this project will be used as a reference are we comfortable with this ?
  • There are two NavMenu view components (one is disabled), which would should be kept ? The other should be removed.

@jahia-ci
Copy link
Contributor Author

Comment by: @romain-pm (Romain Gauthier) on Mon Feb 26, 2024 edited

Review has been done. Next step is to implement the improvements.

@jahia-ci
Copy link
Contributor Author

Comment by: @Janin-Michel-Mathias (Janin Michel-Mathias) on Wed Feb 28, 2024 edited

  • Removed the ids
  • Added the absolute area for navMenu
  • Couldn't fix the webpack config but @jahia-ci (Serge Huber ) has a ticket for that
  • https://jira.jahia.org/browse/BACKLOG-22335 is not yet implemented
  • Integration tests are working, if we want to add tests maybe we should create a ticket for that
  • I didn't see anyproblem with sass/scss except with the webpack config (deployed twice when updating scss) but should be fixed soon
  • Chosen the area in which we can add the navMenu instead of the navMenu itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Jira:SE Jira project this issue was imported from Priority:Unspecified Priority of the ticket Scope-change Task team-goat
Projects
None yet
Development

No branches or pull requests

2 participants