? Pending

User tests: Successful: Unsuccessful:

avatar AndySDH
AndySDH
26 Mar 2020

Summary of Changes

Before this PR, the frontend "edit" layout (edit form) of the user profile (com_users), would also load in the background the fields output that is only meant to be displayed in the frontend result of the profile.

This PR makes sure the code to get the fields output is only loaded in the actual output profile page, not in the input edit form.

Before result

Loading the edit form is slower because it's actually loading the fields output as well, despite not using them

After result

Loading the edit form is faster because it doesn't load unnecessary data in the background

Documentation Changes Required

None

avatar AndySDH AndySDH - open - 26 Mar 2020
avatar AndySDH AndySDH - change - 26 Mar 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 26 Mar 2020
Category Front End com_users
avatar AndySDH AndySDH - change - 26 Mar 2020
Title
[com_users] Fix for fields output being loaded in edit form
[3] [com_users] Fix for fields output being loaded in edit form
avatar AndySDH AndySDH - edited - 26 Mar 2020
avatar HLeithner
HLeithner - comment - 2 Apr 2020

I not sure if this is a good idea, if you have a template override or your own layout it will no longer load the fields...

avatar AndySDH
AndySDH - comment - 2 Apr 2020

Well then maybe it could be done as $this->getLayout() != 'edit' instead.

The problem is that the edit profile and the output profile are sharing the same view for some reason. Unlike com_content for example, which has separate views for article, form, etc. Here the edit form and the output are sharing the same view, just different layouts. This is a bad approach as it causes issues like this.

avatar HLeithner
HLeithner - comment - 2 Apr 2020

And what exactly issue except that rendering the layout took longer?

avatar HLeithner
HLeithner - comment - 2 Apr 2020

Do you have numbers? i mean we need it for default which normally is loaded before edit? We have the same performance impact on this screen.
Conceptional it maybe would better to load fields lazy when it's requested by the layout by this would effect every layout override using custom fields.

Conce

avatar AndySDH
AndySDH - comment - 2 Apr 2020

"i mean we need it for default which normally is loaded before edit"
What do you mean?

The issue is that the code that renders the fields output is loaded in the background also when you are requesting the edit form. With a lot of custom fields involved this will slow down the loading of the edit form page.

avatar HLeithner
HLeithner - comment - 2 Apr 2020

I notice now that your PR has not much to do with custom fields.

I close this PR because you try to remove the possibility to prepare the content for user profiles and it doesn't matter if this is on the default page or the edit page. If a plugin prepares user profile output it's likely that it's also wants to change the edit page.

avatar HLeithner HLeithner - close - 2 Apr 2020
avatar HLeithner HLeithner - change - 2 Apr 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-04-02 12:24:29
Closed_By HLeithner
Labels Added: ?
avatar AndySDH
AndySDH - comment - 2 Apr 2020

"If a plugin prepares user profile output it's likely that it's also wants to change the edit page"

What? What does that even mean? No, nothing of the profile output is used in the edit page at all

avatar HLeithner
HLeithner - comment - 2 Apr 2020

But could be

avatar Hackwar
Hackwar - comment - 2 Apr 2020

FYI: Your PR would break several third party profile extensions and specifically 2 sites of customers of mine. We actually do need those events.

avatar AndySDH
AndySDH - comment - 2 Apr 2020

Ok then nevermind, can you please elaborate why they would break and why those events are needed? Curious to understand it.

avatar AndySDH
AndySDH - comment - 2 Apr 2020
avatar AndySDH
AndySDH - comment - 1 Nov 2020

Got back to this issue, @Hackwar care to elaborate on the question above? :) Thanks!

Add a Comment

Login with GitHub to post a comment