[Enhancement]: Remove Legacy Skin Objects
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:
- Do we agree?
- 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
Related/reference: https://docs.dnncommunity.org/content/tutorials/themes/creating-themes/create-container/
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.
If you're going to move forward, I would turn the task list items into sub-tasks. Thanks!
@bdukes there currently is no error, but most don't work. As an example, these links just do a postback and nothing changes
@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?
👍🏻both of those sound like good ideas
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.
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?
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?
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.
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. :)
Because ASP.NET is trying to compile
_registers.ascxand 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..
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"
That might be an even better idea actually... Way harder to miss.