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

Cms-Kit: Allow to write global css/js #11705

Merged
merged 9 commits into from
Mar 4, 2022
Merged

Cms-Kit: Allow to write global css/js #11705

merged 9 commits into from
Mar 4, 2022

Conversation

yekalkan
Copy link
Member

Resolves #11690

image

@hikalkan hikalkan requested review from hikalkan, EngincanV and gizemmutukurt and removed request for hikalkan March 3, 2022 05:14
@hikalkan
Copy link
Member

hikalkan commented Mar 3, 2022

@yekalkan please help to @gizemmutukurt to test this.

@hikalkan hikalkan removed the request for review from EngincanV March 3, 2022 05:41

public static class GlobalResources
{
public const string Default = GroupName + ".Menus";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public const string Default = GroupName + ".Menus";
public const string Default = GroupName + ".GlobalResources";

@@ -0,0 +1,7 @@
// This file is part of GlobalResourcesAdminClientProxy, you can customize it here
Copy link
Member

Choose a reason for hiding this comment

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

Is this file accidently added? There is GlobalResourceAdminClientProxy above.


}

public Task<GlobalResource> FindByName(string name,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public Task<GlobalResource> FindByName(string name,
public Task<GlobalResource> FindByNameAsync(string name,

{
}

public Task<GlobalResource> FindByName(string name,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public Task<GlobalResource> FindByName(string name,
public Task<GlobalResource> FindByNameAsync(string name,

@@ -0,0 +1 @@
<script src="/global-resources/script"></script>
Copy link
Member

Choose a reason for hiding this comment

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

/global-resources/script is too common. change to /cms-kit/global-resources/script. Also, I didn't see, which controller matches this URL?

@@ -0,0 +1 @@
<link rel="stylesheet" href="/global-resources/style"/>
Copy link
Member

Choose a reason for hiding this comment

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

/global-resources/style is too common. change to /cms-kit/global-resources/style .


var service = volo.cmsKit.admin.globalResources.globalResourceAdmin;

$('#SaveResourcesButton').on('click','',function(){
Copy link
Member

Choose a reason for hiding this comment

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

If we have a single save button, why we have separate service methods (setGlobalStyle and setGlobalScript), and we have only one method for get.
I think this is not consistent. Unify these two methods into a single method and make a single AJAX call.

@hikalkan
Copy link
Member

hikalkan commented Mar 3, 2022

@gizemmutukurt that's OK for me. Please you visually test and merge.

@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

❗ No coverage uploaded for pull request base (dev@84bf31b). Click here to learn what that means.
The diff coverage is 18.24%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev   #11705   +/-   ##
======================================
  Coverage       ?   49.32%           
======================================
  Files          ?     3103           
  Lines          ?    92042           
  Branches       ?        0           
======================================
  Hits           ?    45399           
  Misses         ?    46643           
  Partials       ?        0           
Impacted Files Coverage Δ
...CmsKit/Admin/GlobalResources/GlobalResourcesDto.cs 0.00% <0.00%> (ø)
.../Admin/GlobalResources/GlobalResourcesUpdateDto.cs 0.00% <0.00%> (ø)
...issions/CmsKitAdminPermissionDefinitionProvider.cs 0.00% <0.00%> (ø)
...n/GlobalResources/GlobalResourceAdminAppService.cs 0.00% <0.00%> (ø)
...main/Volo/CmsKit/GlobalResources/GlobalResource.cs 0.00% <0.00%> (ø)
...lo/CmsKit/GlobalResources/GlobalResourceManager.cs 0.00% <0.00%> (ø)
.../GlobalResources/EfCoreGlobalResourceRepository.cs 0.00% <0.00%> (ø)
...ongoDB/Volo/CmsKit/MongoDB/CmsKitMongoDbContext.cs 29.41% <0.00%> (ø)
...B/GlobalResources/MongoGlobalResourceRepository.cs 0.00% <0.00%> (ø)
...CmsKit/Public/GlobalResources/GlobalResourceDto.cs 0.00% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84bf31b...bf85ceb. Read the comment docs.

@gizemmutukurt gizemmutukurt merged commit 03e1796 into dev Mar 4, 2022
@gizemmutukurt gizemmutukurt deleted the issue/11690 branch March 4, 2022 08:45
@gizemmutukurt
Copy link
Contributor

Test is OK.

@gizemmutukurt gizemmutukurt restored the issue/11690 branch March 9, 2022 04:56
@masum-ulu masum-ulu deleted the issue/11690 branch September 27, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to write global css/js for CMS Kit module
3 participants