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

Remove unnecessary function calls for page category and subcategory #116

Merged
merged 1 commit into from Apr 15, 2020
Merged

Remove unnecessary function calls for page category and subcategory #116

merged 1 commit into from Apr 15, 2020

Conversation

bramley
Copy link
Contributor

@bramley bramley commented Apr 4, 2020

When using xdebug I noticed that the _topMenu() function was using 25% of the time to display the home page.
webgrind

Looking into that function showed a coding mistake and unnecessary repeated calls of some functions.

  1. $_GET['pi'] is always set which means the assignment on line 92 is never executed, causing the menu sometimes not have any open section.
    As an example go to Config > Admins then select an admin. The Config section of the menu is closed instead of remaining open.
    Need to compare with an empty string to determine whether the current page is a plugin page.

  2. Related to 1) the pageCategory() function is called for each iteration of a 2-level loop. It can be called only once to find the category of the current page.

  • Lift the function call entirely outside of the loops.
  • Also lift the test for whether the current menu category should be open out of the inner loop.
  1. the function pageSubCategory() can be replaced by simpler code which avoids repeated calls to the function.
    The function finds the subcategory for the current page. The processing then tests whether that subcategory is the same as the current menu page.
    This can be replaced by simply testing whether the current page belongs to the list of pages for the current menu page, if that exists.

There are two sections of code that refer to the "Account" category - lines 97 and 120 in the original file. I guess this applies to phplist hosted, so I'm not able to test that the changes do not break that.

@michield michield self-requested a review April 14, 2020 10:03
Copy link
Member

@michield michield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this locally and it all works fine. @suelaP do you want to proceed with accepting and including this PR?

@suelaP suelaP merged commit 23300dd into phpList:master Apr 15, 2020
@bramley bramley deleted the menu_open_active branch December 17, 2020 11:13
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.

None yet

3 participants