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

Show all themes on page add/edit #2964

Merged
merged 4 commits into from
Jul 13, 2023
Merged

Conversation

markdav-is
Copy link
Contributor

Found a small typo what would limit the theme dropdowns on page add/edits to show only the default theme.

@markdav-is markdav-is changed the title Show al themes on page add/edit Show all themes on page add/edit Jul 1, 2023
@sbwalker
Copy link
Member

sbwalker commented Jul 1, 2023

@markdav-is this is actually not a typo - it was a deliberate change in 4.0. There is a long thread about this topic but the summary is that allowing completely different themes to be used on different pages within the same site has a lot of technical problems which cannot be resolved (due to SPA characteristics). It only works in scenarios where the themes are using the exact same version and source of JavaScript and CSS libraries (like the default Oqtane themes). However if themes use a different version/source of these JavaScript libraries, there will be conflicts and the site will break. Therefore a change was made so that you can only use themes from the same theme package within a site.

@sbwalker
Copy link
Member

sbwalker commented Jul 1, 2023

This is the entire thread:

#2912

There were problems with both CSS and JavaScript for this scenario - so it was best to prevent the behavior by restricting the ability to choose different themes within a site.

Obviously you can still switch themes by changing them in the Site Settings.

Also... if you create a theme which contains theme components that internally use completely different CSS and JavaScript, the framework is not going to prevent you from assigning these theme components to pages as they are part of the same theme. However you should be aware that they may exhibit JS and CSS issues.

@sbwalker
Copy link
Member

sbwalker commented Jul 8, 2023

@markdav-is closing this PR as I explained the expected behavior above in 4.0

@sbwalker sbwalker closed this Jul 8, 2023
@markdav-is
Copy link
Contributor Author

Thanks for the explanation. I'll check out that thread. We use the built-in theme for the admin pages and dashboard. We use a non-boostrap theme for the actual apps. I feel like we can have two themes as long as they don't both use boostrap. With this change, I feel like we need to re-build all the admin UI in our custom theme. So our themes will need to be "complete" and not just for apps. This will add a lot of opportunity costs to our projects.

@sbwalker sbwalker reopened this Jul 13, 2023
@sbwalker
Copy link
Member

@markdav-is I am going to revert the behavior and allow overriding themes at the page level. I will simply add a warning message to the user.

@sbwalker sbwalker merged commit 08438c7 into oqtane:dev Jul 13, 2023
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.

2 participants