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

Fixes #2627 #2631 #2630 #2629 #2632 #2635 - Dev Branch Module and Theme Template Issues #2628

Merged
merged 8 commits into from
Mar 5, 2023

Conversation

thabaum
Copy link
Contributor

@thabaum thabaum commented Mar 4, 2023

Dev Branch Module and Theme Template Build Fix
Fixes #2627
Fixes #2631
Fixes #2630
Fixes #2629
Fixes #2632
Fixes #2635

@thabaum
Copy link
Contributor Author

thabaum commented Mar 4, 2023

Fixed all issues I found while testing out creating a module with Oqtane.Framework-Dev branch. Do I need to make any of these for Master or is it ok just to push them out to dev branch? Dev branch is new but what I was going to use to test some stuff out for fun... I have not tested Theme creation yet however found a similar issue with root folder for Oqtane.Server so applied the same fix to the Theme creator.

I did test the Module theme build and it works with these fixes applied no matter what folder name Oqtane root has.

I am still working my way around it, these fixes just made it so the VS project would build inside the module solution folder.

@thabaum thabaum changed the title Fixes #2627 - Removed extra { } from module template Fixes #2627 #2631 #2630 #2629 Found Dev Branch Module and Theme Template Issues Mar 4, 2023
@thabaum thabaum changed the title Fixes #2627 #2631 #2630 #2629 Found Dev Branch Module and Theme Template Issues Fixes #2627 #2631 #2630 #2629 #2632 Found Dev Branch Module and Theme Template Issues Mar 4, 2023
@thabaum
Copy link
Contributor Author

thabaum commented Mar 4, 2023

These commits fixed all issues I found that caused bugs while using Dev branch to create modules and themes from within the Oqtane Framework.

@thabaum thabaum changed the title Fixes #2627 #2631 #2630 #2629 #2632 Found Dev Branch Module and Theme Template Issues Fixes #2627 #2631 #2630 #2629 #2632 #2635 - Dev Branch Module and Theme Template Issues Mar 4, 2023
@thabaum
Copy link
Contributor Author

thabaum commented Mar 4, 2023

I can add the localization to the template as well, maybe a bit more to it than what is here so I want to let this PR settle in first since these fixes are important to push through. Then I will work on the next patch with the theme settings and resource files if desired for the default theme template creator.

If a great out of the box experience for new developers to Oqtane is desired I think all this PR will help!

@sbwalker
Copy link
Member

sbwalker commented Mar 5, 2023

I am going to merge this PR as the majority of the items it contains are valid - however I do not believe the CSS modifications to the Theme template are valid so I will likely override them with a subsequent PR. In general it is best practice to not bundle many unrelated modifications in a PR as it makes them much more difficult to review and merge - which means they are more likely to be rejected.

@sbwalker sbwalker merged commit 7871f0f into oqtane:dev Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment