gantry / gantry5

:rocket: Next Generation Template / Theme Framework
http://gantry.org
1.03k stars 206 forks source link

Outline Assignment quickly becomes unwieldy / infeasible #1279

Open AlwynBarry opened 8 years ago

AlwynBarry commented 8 years ago

In Wordpress all posts, pages, taxonomies, etc are considered as potential points for Outline assignment. This rapidly results in an unwieldy Outline assignment page, and the site only needs a moderate number of posts before max_input_vars needs to be raised to a large number and the update script times out.

A solution would be to structure the Outline Assignment page. Different 'categories' of post are already identified within separate boxes. These could become separate assignment submission pages to reduce the number of updates at one time - the top-level 'categories' of post would be listed on the page, and selecting one could direct to a page or a modal dialog with the posts displayed on that page/dialog to select/deselect. Within these, 'paging' of large numbers of any one post type would reduce the number of changes submitted at once.

(see discussion here: http://www.rockettheme.com/forum/wordpress-theme-isotope/258649-major-fail-isotope-doesn-t-cope-with-outline-assignment-for-large-sites?start=0#1291819)

newkind commented 8 years ago

We're aware of this issue and we'll try to figure it out in a one or another way asap.

w00fz commented 8 years ago

Hi @AlwynBarry, like Jakub has mentioned we are aware of the issue but we haven't had it tracked in here, so let me first thank you for that.

We have had a discussion internally about how to address it and decided we are going to resolve it in two steps. Let me recap it for you and for the records.

The first step is going to be resolving the max_input_vars problem, we reckon that is bad error and makes assignments potentially unusable when the amount of items is large. We are now going to only ever save 'checked' assignments rather than both checked and unchecked. This alone will cut down in the post request that happens during save and unlikely ever trigger the max_input_vars limit again. This first step is going to be addressed ASAP and we are planning on releasing a new version of Gantry later this week.

The second step is going to be a bit more tricky to realize so we are going to postpone it for a later release. We do realize wordpress' assignable items can be a very huge amount, causing all kind of issues like making the Assignment page loading very slow or adding a lot of extra scrolling to the page which is not ideal. This has actually been bugging us for long as well and we just could never find a better solution. What we agreed on doing now is to add pagination for each 'card' group. Each category/section will only list a fixed amount of items (say 10) and will display a pagination when needed. All checked items will be displaying at the top of the list and will be always available for review and unchecking. With this approach we think the page size will get trimmed quite a lot as well as making everything look cleaner and better organized.

I hope we could get to the step two soon enough, in the meantime stay tuned for the step one and more critical fix, I will notify in here when this happens and you will be able to try a CI Build.

Thank you.

AlwynBarry commented 8 years ago

Thanks for looking at this ... it was a 'show-stopper' for me, and I'm amazed it hasn't been for others, though perhaps folk do most of their Outlines before a lot of content gets added, whereas I moved the content from Joomla beforehand.

The immediate fix of only submitting the changes is certainly a great step - thanks!!.

The longer term fix sounds good - making the initial outline assignment page a page where the 'cards' are identified and then you browse down from there to assign within the cards is the right solution. The big problem that I foresee is if someone decides to change (say) all posts to a new outline ... with any blog this could quickly amount to hundreds and you'd quickly get to the max_input_vars problem arise again unless you captured the changes in an array of values that are passed in a single variable (for example). If you have hundreds of posts it would be frustrating not to be able to make this change at the top-level 'switch' but have to go through the pages below the card level, so this is the obvious 'edge case' to cater for. Apart from that, this solution should provide a much better approach to the problem.

For your info, although it is a particular problem with Wordpress, I did face a similar difficulty in Joomla where the problem was the script timing out rather than the max_input_variables when I changed from one outline to another for the majority of my pages. I ended up having to change the original outline, which I didn't want to do because I wanted the opportunity to go back if the change wasn't effective for the users. So, a solution will be useful for Joomla users too, I guess.

w00fz commented 8 years ago

Ok, part 1 of the issue has been now resolved and it is available for testing in our CI Builds. We are still planning a new release of Gantry tomorrow or early next week.

Cheers!

newkind commented 8 years ago

About the page size - we had an implementation of the max-height of the cards and vertical scroll bar in each of them, so all cards would have the same maximum height (or less) :

isf4j51rlfcaol0xdayd9axgdhi5lb

Unfortunately we had to disable it because of a flexbox rendering bug in Chrome which has been there for the past 5 or 6 (sic!) Chrome releases!

I just tested latetest Canary build and I can see that they finally started fixing it. It's still not ideal and causes some rendering issues (before that it was just empty white card), but hopefully it'll get sorted out when it goes stable.

r5aeakaeitocc3ueysfobng9x3qjcf

AlwynBarry commented 8 years ago

Ok, I've tried the CI build and I can now assign layouts to pages! The pause after each time an item is selected remains long, but at least I can work with layouts again! Thanks so much!! I can't test it extensively because I don't have that many different layouts, but at the moment it appears to work as expected.

AmibeWebsites commented 8 years ago

ETA on releasing the first part of the fix into the wild?

w00fz commented 8 years ago

Oh you must of missed it. It's already in the 5.2.18 release.

mahagr commented 8 years ago

Created a followup issue as 5.2.18 milestone has already been closed.

AmibeWebsites commented 8 years ago

@w00fz Oh dear :-( I'm using 5.2.18 and get a 400 Bad Request. This is a site I'm migrating into Wordpress where there are four custom post types and well over 1000 posts, pages, menu items and categories combined. The max_input_vars is 1000.

p.s. I did check the 5.2.18 change log and didn't see it there.

mahagr commented 8 years ago

Where are you getting this 400 response? If assignments fail to save, it should have error which asks you to raise max_input_vars value. If the error is something else, you have likely ran into another issue, which may be unrelated to this issue.

AmibeWebsites commented 8 years ago

I get it when I try to save assignments and yes, it asks to raise max_input_vars.

AlwynBarry commented 8 years ago

It's working fine for me still ... updated to 5.2.18 and no errors. It is still VERY slow to select, but seems to save Ok without error. However, I guess it would still give an error if I was changing the default page, which would then change many allocations at a time. I'm just careful to only change a few allocations each time.

My main problem now is that there are saved allocations that aren't being implemented when the theme is rendered. I entered a new 'Contacts' layout and assigned the 'Contact Us' page and menu item to that layout. All saved Ok, but the 'Contact Us' page is still using the Default layout. With the current system of assignments I don't know if there is another assignment of some term or other that is overridding the settings, or whether it's an error in the allocation (when I go back to the allocation page, however, the allocations have been recorded correctly). My thinking at the moment is that there is another setting that is overriding the page setting (maybe for the category term) because on some other pages I have had similar problems when there were a number of settings that could apply to the same page. Sadly, Gantry provides no clear 'hierarchy' of settings that lets you see when you've changed all that are required.

Having used other Wordpress themes, in which you allocate to pages or posts in the Page editor, I have to say that the Gantry method is much less intuitive for individual page/post allocation. It works well with Joomla, but just doesn't really fit the Wordpress way of doing things.

w00fz commented 8 years ago

Even though Matias opened a separate issue with #1320, I'd rather reopen this issue and change milestone, since the discussion has been going on in here.

newkind commented 8 years ago

@AlwynBarry Menu Item Assignments are working by comparing current page URL to the menu item URL and they need to match in order to work. Are you sure that you don't have any extra parameters in the menu item url or maybe you're using any language switcher plugin ? I'm sure there must be cause why it wouldn't work for you.

AlwynBarry commented 8 years ago

I've checked and the page URL and menu URL are the same ... no extra parameters, and no language switcher involved. The only thing that is different is that permalinks are set to %category%/%postname% but the menu picks that up fine - join-us/contact-us. The page slug is contact-us, but I've assigned both the page and the menu item to the outline. (I've also tried all combinations of setting either or both of these!).

Looking in the assignments.yaml file for the assigned Outline we have:

context: {  }
menu:
  main-menu:
    3027: true
post:
  page:
    3024: true
taxonomy: {  }
archive: {  }

Neither of these numbers appear in the default assignments.yaml and so the assignments have been made correctly by the script.

Querying the site with URL .../?p=3024 gives the page join-us/contact-us/ and looking in the database 3024 is the post with postname contact-us and database item 3027 is the nav_menu_item post type.

So, I can't see any problem with the assignment script or the name or the menu entry. Any other ideas? I guess you can see why I said that the way Gantry does the assignments is rather unintuitive in Wordpress because there are potentially category assignments that could (I guess) override the page assignments ... but there's no way of telling if that's whats happening.

So, in the redesign, the 'hierarchy' of assignment decisions needs to also be identified, I think.

But all the above is slightly 'off topic'. The original problem was the problem with max_input_vars and the unweildy nature of the assignments page. This is now a problem with the transparency of how the assignments operate through to what is seen in the page rendering, which you may see as 'off topic'.

mahagr commented 8 years ago

I remembered that there's issue for this, but maybe its closed as I cannot find it.

You can see the decision by following code:

$assignments = new Gantry\Framework\Assignments;
print_r($assignments->scores());

You can also open up the decision:

print_r($assignments->matches());
newkind commented 8 years ago

Please check what @mahagr wrote and if this won't give you a hint, you can ping me in Gitter and I can try to find a solution for you.

nikola3244 commented 8 years ago

@newkind, scrolling problem disappears when you remove -webkit-backface-visibility: hidden; and backface-visibility: hidden; from <label>. I don't see any difference when it's there and when it's not.

In Chromium 51.0.2704.84 (64-bit) https://drive.google.com/file/d/0B8fB_QtQf14TQUFEN1FaaGM5eW8/view?usp=sharing

Edit: This was recorded in 5.2.18

w00fz commented 8 years ago

Probably best to test with CI Builds, there's been a lot of changes in the SCSS about assignments, I might already have fixed that.

newkind commented 8 years ago

@nikola3244 this should be fixed already in latest CI and Chrome stable - 51.0.2704.103 (64-bit)

nikola3244 commented 8 years ago

True, it's fixed.

AlwynBarry commented 8 years ago

Finally got chance to test the suggestion of @mahagr

The output for the outline choice was:

Array
(
    [sbc_contact] => 3
)
Array
(
    [sbc_contact] => Array
        (
            [menu] => Array
                (
                    [main-menu] => Array
                        (
                            [3027] => 3
                        )
                )
        )
)

But, it's still displaying the default template! So, the assignment is correct, the decision is correct, but the output is not! I really can't work this one out!!

But, I'm back to the original problem ... I've moved to the production site and I don't have access to the max_input_vars setting and now, even with the up to date Gantry 5, it always gives me a max_input_vars error. So either the fix wasn't carried through in the current releases, or it still needs a higher than default max_input_vars setting even when only changing one item.

Sticking with Gantry to get the site going, but can't stick with Gantry 5 long term. Such a pity - it worked like a dream in Joomla!

mahagr commented 8 years ago

You're right, the fix doesn't seem to work in WP but It does work in Joomla.

CC: @w00fz

w00fz commented 8 years ago

Sorry, you are right, for some reason my implementation was only applied to Joomla. I fixed this now in the currente develop, you can try the latest CI Build and it should be working as discussed above.

mahagr commented 8 years ago

I don't think that the issue is fully fixed (nor that it was intended to fix from the commit) -- we still need to start using ajax to make the page easier for the poor browsers.

AlwynBarry commented 8 years ago

I've applied the CI and can change the assignments without error. However, two problems:

So, no real solution still, though having a stable with the assignments fix and which doesn't cause a WSOD would be a step forward at least!

mahagr commented 8 years ago

For me everything works, so it may be hard to track down. Which version of PHP are you using? Do you have opcache enabled? Are you seeing anything in Apache error.log?

nikola3244 commented 8 years ago

@AlwynBarry Also, enable WP debug mode. You might see some errors that could point to the cause of the issue.

mahagr commented 7 years ago

This issue affects Joomla too: #1768

A member had rasied a topic in the forum here: http://www.rockettheme.com/forum/joomla-template-interstellar/265856-mass-changing-menu-assignments-browser-hangs

Basically they wish to assign/deassign 200+ menu items in Gantry 5 admin on the assignments tab and the main.js is taking a very long execution time. In fact, in Firefox, you get a popup every minute or so saying that the script may have stopped responding and you have to press the "continue" button.

After many times of pressing "continue", or checking "do not tell me again" eventually the script will complete successfully and the assignments/deassignments will work correctly.

AmibeWebsites commented 7 years ago

A thought on this, as a temporary workaround... it is very seldom (probably never) that I assign a layout to an individual post - a page yes (talking WordPress here), but not a post. Posts are like news items or products and so I'm far more likely to assign a layout to a category or post type.

And since it's mostly posts (don't often have over 1000 categories) that cause the sloooooooooowness, could there be a setting "Display posts for assignment" set to False by default?

What are others experiences with assigning to individual post?

AmibeWebsites commented 7 years ago

Dirty, but fast workaround...

Copy the file admin/templates/pages/configurations/assignments/assignments.html.twig to your theme

Add the following condition within the {% for name, list in types %} loop:

{% if not name or name not in ['post', 'shop_order', 'shop_coupon', 'shop_webhook', 'link_category', 'attachment'] %}
  ...

Add the following condition within the {% for link in list.items %} loop:

{% set continue = continue|default(1) and link.label != 'Single Posts' %}
{% if continue %}
  ...
mahagr commented 7 years ago

I just added Gantry options for removing posts and pages (= all but posts) into CI build, please test it out.

igor-gp commented 6 years ago

Wow i have just hit this issue myself with a website and it is sooo slow. It is not usable.

I am in WordPress, latest version of everything. @newkind any way we can speed this up so we can use it?

I have turned off Posts in the settings but that hasn't really helped.

newkind commented 6 years ago

Nothing really we can do about it without some bigger changes. Not sure when that happens.

alexbangle commented 6 years ago

The workaround above actually worked for me by adding in some rules to filter out some stuff {% if not name or name not in ['attachment-terms','page-terms','post-terms','post'] %} but I had to first remove the if statement from a more recent update {% if list.items %} Seems like the long term fix would be a way to have a way to remove sections from assignments but for now this at least gets the site running. To find the names of the types to remove I just inspected the source on the assignments page.