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

Core Enhancement 1 - Place Module in Menu #1

Open
wants to merge 77 commits into
base: dev
Choose a base branch
from

Conversation

YatharthVyas
Copy link
Collaborator

@YatharthVyas YatharthVyas commented Jun 5, 2021

The purpose of this PR is to track all the file changes and commits made. Mentors can review the changes and provide feedback.
I will update the description as and when I push more commits.

Plan Document: https://docs.google.com/document/d/1Pl8JGa2hkYkmJzQOn9_mS8a4imDmqc2a/edit#heading=h.gewhdn9rqyeh


Testing Instructions


Pull Request for

  • Rendering Add Module Button in Frontend via Plugin
  • Setting up ?pm=1 and rendering buttons and positions for it
  • Passing data of position and menu assignment (pages) from the button to Add Module page
  • Adding Preview link in backend’s add/edit module page
  • Setting up the Modal for Backend
  • Setting up the frontend view for the Modal and returning the data

Todo

  • Add a message to inform users that they need to login and they need permissions for ?pm=1 view

Result After PR

Overall Flow (Latest)

https://drive.google.com/file/d/1xaiEEZfUkE_6p7jCZLss9udkjJiJKJnS/view?usp=sharing

Frontend Flow

Frontend.Placements.mp4

Backend Flow

modal


Summary of Changes

  1. Switcher Added in Global Configuration for Templates
  2. Added the code in SQL Installation Script to set the default value of this Parameter upon the first installation
  3. Add Module Button is displayed in <main> in Frontend when the above switcher is saved as 1 or True

pr11-gs


  1. Set up the view for ?pm=1 in Frontend (Here pm stands for Place(ing) Module)
  2. The button in (3) is linked to this view
  3. The preview shows the Add Module Here button for every position

pr2-gs

  1. The Add Module Button Works for Every Menu Page (not just index.php):

pr111-gs

  1. The Add Module Here Button is accessible as it also dictates the position:

image


9. Check for Permissions before displaying Place Modules Buttons for displaying the Add Modules Button and accessing the `?pm=1` view

pr1-gs-perms


  1. Work in Progress on a Controller to set Position by passing data from the button. It displays Menu ID, template ID and name of the position.

pr1-gs-addPost

  1. Template ID in (10) is not required (I checked the databases and Models of Module's Add Methods) so it has been removed.

  1. Rephrased Add Module Here button to Place Module Here to avoid confusion and generalize it for the Backend Preview Modal that we plan to implement.

  2. Removed the HTML tags from language constants

image


  1. Position is now pre-filled based on Frontend Placement Selection

pr1-gs-posi


  1. Menu Position is now pre-selected
  2. Made a few semantic changes

Potential Issues

  1. There is a flaw in the get active menu item API for a multilingual website:

image

https://docs.joomla.org/Menu_and_Menuitems_API_Guide#Active_Menuitem

@YatharthVyas
Copy link
Collaborator Author

I apologize for the review requests. It happened unintentionally. Sorry to disturb all of you.

@bembelimen
Copy link
Contributor

Not your fault, happens automatically as it's in the settings.

@YatharthVyas
Copy link
Collaborator Author

YatharthVyas commented Jun 26, 2021

The latest commit fixes Issue #6 but it's not perfect, there is still an extra toolbar at the top (For Frontend only, Backend is fine).
This could be removed by adding &tmpl=component to the URL but it causes an Issue that is, if the user isn't logged in to the backend while opening the modal then it redirects the user to the Admin Dashboard Home page after logging in. (Correct Behavior would be to redirect to the modal view but tmpl=component somehow overrides it)

image

@YatharthVyas
Copy link
Collaborator Author

The recent set of commits fix #5
And introduces a minor improvement in the Module Position View that no longer displays the card style:
image

@brianteeman
Copy link
Contributor

This really must be generic so that it will work with all templates and not require any changes to the template itself

@YatharthVyas
Copy link
Collaborator Author

YatharthVyas commented Jun 27, 2021

This really must be generic so that it will work with all templates and not require any changes to the template itself

Agreed.
So as of now, the only template (Cassiopeia) specific files that are modified by this PR are:

  • Index.php: A condition has been added to display all positions even if the count of Modules for that position is Zero. This condition is only for the ?pm=1 (place module in position) view (Comment)
  • New SCSS File in Cassiopeia: The styling of the "Add Module to this Menu" button is a part of Cassiopeia's CSS because the border color of this button depends on the template's primary color var (Comment)

Everything else is general and should work smoothly for all templates.

@brianteeman
Copy link
Contributor

Index.php: A condition has been added to display all positions even if the count of Modules for that position is Zero.

That still makes this core feature dependent on the template :(

@YatharthVyas
Copy link
Collaborator Author

I thought about the template file dependency a lot but I still cannot think of any other way to do it that would be correct.

The changes in this file is more like an improvement. If any 3PD does not do the changes then this feature will still work, the only difference will be that it will only show the active positions that have a module count >= 1

It's not perfect but I hope this is fine :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants