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

Suppress Edit Bar #4038

Closed
meetmandeep opened this issue Sep 2, 2020 · 19 comments · Fixed by #4070
Closed

Suppress Edit Bar #4038

meetmandeep opened this issue Sep 2, 2020 · 19 comments · Fixed by #4070

Comments

@meetmandeep
Copy link
Contributor

Description of problem

It's easy to swap out the Control Panel by updating the Host Settings; however, there is no way to change the Edit Bar experience.

Currently, the Edit Bar listens for SkinEvents via an HttpModule and automatically injects itself for each page load (even for anonymous users): https://github.com/dnnsoftware/Dnn.Platform/blob/develop/Dnn.AdminExperience/EditBar/Dnn.EditBar.UI/HttpModules/EditBarModule.cs#L62

The Edit Bar (UserControl) then removes itself it it's not need (not in Edit Mode): https://github.com/dnnsoftware/Dnn.Platform/blob/develop/Dnn.AdminExperience/EditBar/Dnn.EditBar.UI/Controllers/ContentEditorManager.cs#L120

This is a performance issue which should also be resolved. The Edit Bar does not get injected if the Application.SKU != "DNN"; however, that is a read-only property that cannot be changed so ultimately there's no way to prevent it from appearing on the page.

Description of solution

Proposed solution is to change the follow line: https://github.com/dnnsoftware/Dnn.Platform/blob/develop/Dnn.AdminExperience/EditBar/Dnn.EditBar.UI/HttpModules/EditBarModule.cs#L62

if (DotNetNukeContext.Current.Application.SKU != "DNN")
{
     return;
}

with:

if (HostSettings["EnableEditBar"] != "TRUE") //Psuedo Code
{
     return;
}

Additional context

We can also introduce a corresponding checkbox in Persona Bar with a note such as "Default Edit Bar is disabled and content editing is managed by a 3rd party component"

Please suggest the best location in Persona Bar to add this setting.

I'll be happy to contribute a PR for this issue. Let me know when we have consensus and I'll get started.

@bdukes
Copy link
Contributor

bdukes commented Sep 3, 2020

I like the idea of a host setting; I guess the control panel setting is a host setting, too, right? I was wondering if it made sense to be a portal setting, but it should correlate to the control panel setting, IMO.

@meetmandeep
Copy link
Contributor Author

@bdukes Yes. ControlPanel is a Host Setting.

@thabaum
Copy link
Contributor

thabaum commented Sep 8, 2020

I agree with a both portal specific and a host setting that may or may not allow changing this setting for portals.

Host Settings:

  • force enable all portals to use third party edit bar
  • ability to allow specific portals ability to use third party edit bar
  • no third party allowed edit bar allowed on any portals

radio just two options one turns it on and off to allow portals but I like the added option to force all to use 3rd party then grey read only the portal setting option when it is on for all or off for all.

Portal Settings:

  • Use Third Party Edit Bar
  • Use Host Defaults

Using a checkbox

In my opinion these would be nice to see available for options.

@bdukes
Copy link
Contributor

bdukes commented Sep 8, 2020

As of right now, the control panel settings is at the host level, so this new edit bar setting should be at the host level, too. We can open a separate issue to discuss whether those should allow overrides at the portal level or not.

@sleupold
Copy link
Contributor

sleupold commented Sep 8, 2020

IMO there is no real need to configure this on site level.

@mikesmeltzer
Copy link
Contributor

My comments are a bit late now.. but I feel that the Edit Bar is a tightly coupled component to the Persona Bar. I think they should be treated as such and not as separate things.

Instead of a separate setting to turn part of the Persona Bar solution off, I recommend just doing a check to see if the Persona Bar is the current selected Control Panel (via the existing host setting) and if so then inject the Edit Bar.

Thoughts?

@meetmandeep
Copy link
Contributor Author

meetmandeep commented Sep 9, 2020

Edit Bar is a tightly coupled component to the Persona Bar

Edit bar is not coupled with Persona Bar. They're separate projects and can be swapped separately. An example of this is Evoq which uses the core Persona Bar (w/Extensions) but a seperate Edit Bar.

@mikesmeltzer
Copy link
Contributor

Yes they are physically separate projects, etc. but I view the relationship a bit differently.

I find that the default implementation is tightly coupled from both a UI perspective (they are built to work exactly with each other) and the snippet of code you posted above that indicates that the edit bar is injected only when the App SKU is 'DNN'. Correct me if I'm wrong but what that is basically saying is that if the current installation is not DNN (meaning Evoq) then do not inject the core Edit Bar since it wouldn't meet the App SKU "DNN" criteria?

I can't see the Edit Bar being used independently without the Persona Bar or swapped out standalone.

@mitchelsellers
Copy link
Contributor

@mikesmeltzer Are you proposing then that we don't do anything other than update this to check if the control panel isn't the PersonaBar bar and hide?

@mikesmeltzer
Copy link
Contributor

@mitchelsellers 100%.

@mitchelsellers
Copy link
Contributor

I guess @bdukes @david-poindexter any thoughts on this?

@meetmandeep
Copy link
Contributor Author

With current approach, you could simply swap out the Persona Bar with a different set of UI to adminster. It has absolutely nothing to do with Editing Experience. IMO, we should stick with the current pattern and not couple the PersonaBar with EditBar.

@mikesmeltzer
Copy link
Contributor

@meetmandeep I can't find any code that allows you to swap the Edit Bar out. It appears that it is static (with the ability to extend the menu items) and always injected if App SKU is DNN.

Can you point me in the direction?

@meetmandeep
Copy link
Contributor Author

I can't find any code that allows you to swap the Edit Bar out.

@mikesmeltzer If there was a way then there would be no need for this issue/PR. That's exactly what this PR solves; allows you to now swap it with your own.

Evoq is swapping it out with their own and not injecting the default one by hardcoding the DNN Sku; but now it's a configurable setting that can be used with any developer.

The more I think about this, the stronger I feel that Edit Bar and Control Panel should remain separate. As an example you could use the Persona Bar for admin UI but create your own edit experience similar to Square Space where you add modles directly in content pane. This is precisely why it was decoupled to begin with so Evoq can change the edit experience.

@meetmandeep
Copy link
Contributor Author

With current approach

By this I meant the solution proposed and submitted in PR; not what's currently available in DNN.

@mikesmeltzer
Copy link
Contributor

@meetmandeep This PR doesn't make it a swappable component but disables the injection functionality. I agree, this itself is an improvement when compared to the current functionality, and I welcome it. It would be great to turn off this injection in some use cases.

However, we'll have to agree to disagree on the coupling discussion. With the Edit Bar, it may be "separate" but it isn't swappable (before or after the PR), was visually designed to work specifically with the Persona Bar, and appears to have hardcoded logic (until your proposed change) to prevent it from loading in Evoq. There is nothing in this, for me, that suggests the Edit Bar is a standalone entity and not "part" of the Core Admin Experience (Persona Bar + Edit Bar but not one without the other).

@mitchelsellers
Copy link
Contributor

@mikesmeltzer You are correct in that Persona Bar + Edit Bar is a requirement for the default edit experience. However, I don't believe it is mutually exclusive that the replacement of one should require the replacement of the other.

Evoq is a great example of this. A future extension point to swap it as a configuration I think would be an option if needed.

The PB in Platform, without the EditBar can do certain things still

@mikesmeltzer
Copy link
Contributor

mikesmeltzer commented Sep 9, 2020

@mitchelsellers I personally see limited use cases to have the Persona Bar turned on without the Edit Bar turned on too, it's the only method of adding modules to pages outside of custom control panels or core control panels prior to DNN 9. To me the Evoq logic feels like might have been more of a work around hardcoded into platform than a feature as it's dependent on the entire install not being DNN.. But that's ok.

The PR is an improvement compared to today's functionality and I'll certainly use it (thanks @meetmandeep) regardless if I disagree on the topic.

FYI: Evoq might be affected by the logic change if I'm understanding it correctly.

@mitchelsellers
Copy link
Contributor

Yes, this is a known change that will impact Evoq.

Moving these to be co-dependent would make the current Evoq model 100% impossible, as well as would break many other providers that are either actively developing or researching development of new functionality. I agree that we have some better options, and that the edit bar itself is a bit of a "interesting" implementation. But this is something that CAN benefit many, and comes with limited risk.

If we want to create another issue and make this a bigger conversation we sure can for a future, 10.x or later version.

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

Successfully merging a pull request may close this issue.

6 participants