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

Tut. > Theme Objects > Add TEXT and move ControlPanel #713 #714

Merged

Conversation

Timo-Breumelhof
Copy link
Contributor

  • Add the TEXT Theme Object
  • Move Controlpanel to legacy as it's not used in DNN 9 anymore
  • Change TOC

Closes #713

@Timo-Breumelhof
Copy link
Contributor Author

@david-poindexter Hmm, not sure how I could have prevented that merge conflict..? Maybe I should have done it in 2 PRs?

@Timo-Breumelhof
Copy link
Contributor Author

@valadas Hmm. "index.ascx.resx" does work (I have it in my test Theme).
And that's what has always been in the documentation.
But I guess both work, I'll try it in my test Theme with index.resx

@david-poindexter
Copy link
Collaborator

@david-poindexter Hmm, not sure how I could have prevented that merge conflict..? Maybe I should have done it in 2 PRs?

Typically this happens due to the current state of the branch from which you based your working branch. If it is not up to date with the latest version, then you could simply get the latest from the base branch and merge it into your working branch.

Alternately, we could try to manually Resolve conficts in the browser. See the red brackets in the below screenshot and determine which branch "wins", or determine what should be the corrected lines there for this file if it is different than the two branches represented.

image

@valadas
Copy link
Member

valadas commented Apr 21, 2024

Hmm, apparently, I am wrong (or this has changed at some point in DNN history). I just tested and you are right about the file name at least in current versions. I could have sworn I had old themes using just the folder name for the file so the localization would be available for multiple layouts having a single source of truth.

Copy link
Member

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong on my requested changes and I resolved the merge conflict. This now looks good to me.

Copy link
Collaborator

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@david-poindexter david-poindexter merged commit a657d3f into DNNCommunity:main Apr 21, 2024
2 checks passed
@Timo-Breumelhof
Copy link
Contributor Author

Hmm, apparently, I am wrong (or this has changed at some point in DNN history). I just tested and you are right about the file name at least in current versions. I could have sworn I had old themes using just the folder name for the file so the localization would be available for multiple layouts having a single source of truth.

Actually that's a valid point. It would be nice if you could have one resx file for all layouts.
Maybe we could add an attribute
public string ResourceFile { get; set; }
For that..

@sleupold
Copy link

@Timo-Breumelhof
afaik you can use "skin.ascx.resx" for your purpose.

@Timo-Breumelhof
Copy link
Contributor Author

@Timo-Breumelhof afaik you can use "skin.ascx.resx" for your purpose.

I also had the idea something like that existed but it does not seem to work.
And I don't see anything for this in the core code either..
https://github.com/dnnsoftware/Dnn.Platform/blob/9367336b703f8251c4c9c09fa09655246cd10513/DNN%20Platform/Website/admin/Skins/Text.ascx.cs#L32

@sleupold
Copy link

ok, maybe I mix it up with skin.css and skin.js.
in this case, supporting skin.ascx.resx (or skin.resx) would make sense.

@valadas
Copy link
Member

valadas commented Apr 22, 2024

Strange, I was also pretty sure there was a way to not have to do a file for each control and there was a way to have a file for the whole theme. I was proved wrong though...

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

Successfully merging this pull request may close these issues.

Add TEXT Theme Object & move Controlpanel to legacy
4 participants