Dnn.Platform icon indicating copy to clipboard operation
Dnn.Platform copied to clipboard

[Enhancement]: Remove Legacy Skin Objects

Open Timo-Breumelhof opened this issue 6 months ago • 14 comments

Is there an existing issue for this?

  • [x] I have searched the existing issues

Description of problem

For the MVC pipline, I'm going through all Skin and Container objects.

I notices some of them do not work (or I could be missing something).

Some examples of the ones I think are legacy as I did not get any of them to work:

Containers:

  • [ ] actionbutton.ascx > Does not work in DNN 9
  • [ ] dropdownactions.ascx > Does not work in DNN 9
  • [ ] linkactions.ascx > Does not work in DNN 9
  • [ ] printmodule.ascx > Does not work in DNN 9
  • [ ] Toggle.ascx > Does not work in DNN 9

Skins controlpanel.ascx treeviewmenu.ascx LinkToFullSite.ascx LinkToMobileSite.ascx modulemessage.ascx

These are already marked as Legacy: on DNN Docs.

https://docs.dnncommunity.org/content/tutorials/themes/theme-objects/index.html

ACTIONBUTTON CONTROLPANEL LEFTMENU LINKS NAV TREEVIEW VISIBILITY STYLES

My suggestion is to make sure they actually don't work and then remove them. I know they have not been marked for removal, but if they don;t work anymore (and are not useful..)

My main questions:

  1. Do we agree?
  2. If so shall I create an issue per SKO so we keep any feedback split up? Or just this issue?

I will push the ones that remain to the documentation at a later stage. (I have an test Open Content template that also generates the MD for dnndocs)

Timo

Description of solution

I suggest this:

  • For all SKOs and/or options that doe not work any more, we add code with logic that renders a warning to Host (maybe even admin?) users in edit mode that says "Them object X is a legacy Theme object, that will be removed in version X " (link to some page explaining what to do)

Description of alternatives considered

No response

Anything else?

No response

Do you be plan to contribute code for this enhancement?

  • [x] Yes

Would you be interested in sponsoring this enhancement?

  • [ ] Yes

Code of Conduct

  • [x] I agree to follow this project's Code of Conduct

Timo-Breumelhof avatar Jun 27 '25 12:06 Timo-Breumelhof

Related/reference: https://docs.dnncommunity.org/content/tutorials/themes/creating-themes/create-container/

jeremy-farrance avatar Jun 27 '25 14:06 jeremy-farrance

If they're used in a theme/container today, do they error, or just do nothing? I wouldn't want to remove them if it turns a silent error into a loud error (until the documented version where we've said they'll be removed and cause an error).

However, if they currently cause an error anyway, I'm fine with just removing them.

bdukes avatar Jun 30 '25 13:06 bdukes

If you're going to move forward, I would turn the task list items into sub-tasks. Thanks!

bdukes avatar Jun 30 '25 13:06 bdukes

@bdukes there currently is no error, but most don't work. As an example, these links just do a postback and nothing changes

Image

Timo-Breumelhof avatar Jun 30 '25 14:06 Timo-Breumelhof

@bdukes so we say they are legacy on dnn docs? We can do that in code but I don't think any Theme developer will see that.. Or what about rendering "This theme object will be removed in the next version of DNN" for admins?

Timo-Breumelhof avatar Jun 30 '25 14:06 Timo-Breumelhof

👍🏻both of those sound like good ideas

bdukes avatar Jun 30 '25 14:06 bdukes

We have discussed potentially adding a javascript warning/error when they are used. It is not perfect but together with release notes and documentation updates, it is the best we can do IMO. Showing it to regular visitors to me sounds a bit strange if we don't do something in advance of that breaking change.

valadas avatar Jun 30 '25 20:06 valadas

Recent related history and experience... in case this gives someone ideas on how this works over time.

Both NAV and LEFTMENU were removed in DNN v10. I know because I tried an upgrade on a site that had them registered in the them, but not used. There was no way to see any warning that they were deprecated and after the upgrade to 10, startup failed with the yellow screen of death.

<%@ Register TagPrefix="dnn" TagName="LEFTMENU" Src="~/Admin/Skins/LeftMenu.ascx" %>
<%@ Register TagPrefix="dnn" TagName="NAV" Src="~/Admin/Skins/Nav.ascx" %>

If you were local (on the server), the error was clear enough (for someone with experience) and just deleting the two lines above fixed the site (theme).

Can anyone think of a way to warn about the deprecation both when a) actually used and b) registered but not used?

jeremy-farrance avatar Jul 02 '25 12:07 jeremy-farrance

I think if one only has the register tag that's not going to be easy? BTW, did that site not revert to the default theme with a "......ascx" not found error?

Timo-Breumelhof avatar Jul 02 '25 12:07 Timo-Breumelhof

Because ASP.NET is trying to compile _registers.ascx and the Src is missing, it immediately gives up (throws the error). If there is a way to catch that type of error and show warning and continue on, I never learned it back in the day.

jeremy-farrance avatar Jul 02 '25 12:07 jeremy-farrance

Just so its mentioned... One option is to write a pre-upgrade app that scans your DNN instance (files and data) looking for things like this (crazy pattern matching fun?). It is not unheard of or impossible, but it does sound like more work than I would expect anyone to be able to contribute.

If it were done well as an installable DNN module, it could be upgraded over time with rule sets so it could be updated and added to. I think installing a module that could reveal issues ahead of the upgrade sounds pretty compelling the more I think about it. :)

jeremy-farrance avatar Jul 02 '25 12:07 jeremy-farrance

Because ASP.NET is trying to compile _registers.ascx and the Src is missing, it immediately gives up (throws the error). If there is a way to catch that type of error and show warning and continue on, I never learned it back in the day.

Hmm, right I see now that only some skin objects revert to the default theme when missing..

Timo-Breumelhof avatar Jul 02 '25 12:07 Timo-Breumelhof

We have discussed potentially adding a javascript warning/error when they are used. It is not perfect but together with release notes and documentation updates, it is the best we can do IMO. Showing it to regular visitors to me sounds a bit strange if we don't do something in advance of that breaking change.

Why not Server side code:

if (Host and Edit Mode) > "This is a legacy SKO that will be removed in version x + help link"

Timo-Breumelhof avatar Oct 01 '25 10:10 Timo-Breumelhof

That might be an even better idea actually... Way harder to miss.

valadas avatar Oct 01 '25 15:10 valadas