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

Feat/calendar component initial #1

Merged
merged 25 commits into from
Jul 23, 2024
Merged

Conversation

bkapustik
Copy link
Collaborator

@bkapustik bkapustik commented Jul 4, 2024

Motivation

Calendar form component integration enabling to add a calendar form component. This integration allows for choice between seelcting a date, date and time or range of days, or multiple days. In addition it allows for simple programatic exclusion of excluded date and time values.

@bkapustik bkapustik requested review from michalJakubis and MartinK-Kentico and removed request for michalJakubis July 8, 2024 09:15
@michalJakubis
Copy link
Contributor

Can you please provide more info to this PR within description? What is this initial PR about? What is complete? What is missing and needs to be added in subsequent PR?

@michalJakubis
Copy link
Contributor

Also please replace or remove all references to Kentico.Xperience.RepoTemplate and check that tasks in .vscode folder works as expected.

@bkapustik bkapustik requested review from michalJakubis and removed request for MartinK-Kentico July 12, 2024 07:56
<PackageVersion Include="SonarAnalyzer.CSharp" Version="9.23.0.88079" />
<PackageVersion Include="Microsoft.Extensions.Logging.Debug" Version="3.1.8" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix indentation


<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems>
<TimestampServerUrl>http://timestamp.digicert.com</TimestampServerUrl>
<NoWarn>$(NoWarn);1591;S3267</NoWarn>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix indentation

@@ -9,9 +9,9 @@
"type": "coreclr",
"request": "launch",
"preLaunchTask": "build",
"program": "${workspaceFolder}/src/Kentico.Xperience.RepoTemplate.Sample/bin/Debug/net6.0/DancingGoat.dll",
"program": "${workspaceFolder}/src/Kentico.Xperience.CalendarComponent.Sample/bin/Debug/net6.0/DancingGoat.dll",
Copy link
Contributor

Choose a reason for hiding this comment

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

This path does not seem to be correct, there is no Sample project. Can you make sure that this task is working as expected?

@@ -71,7 +71,7 @@
"watch",
"run",
"--project",
"${workspaceFolder}/src/Kentico.Xperience.RepoTemplate.Sample/DancingGoat.csproj"
"${workspaceFolder}/src/Kentico.Xperience.CalendarComponent.Sample/DancingGoat.csproj"
Copy link
Contributor

Choose a reason for hiding this comment

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

The same issue as above.

@@ -0,0 +1,33 @@
{
"name": "ktc-calendarComponent",
Copy link
Contributor

Choose a reason for hiding this comment

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

@kentico/xperience-calendar-component

Copy link
Contributor

Choose a reason for hiding this comment

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

The name of this file is no longer valid. The package should be called xperience-calendar-component. We need to rebuild Client package with correct name set in webpack config and include the latest one with correct name.

@@ -7,7 +7,7 @@ const config = {
entry: "./src/index.js",
output: {
path: path.resolve(__dirname, ".././wwwroot/js"),
filename: "ktc-calendarComponent.js",
filename: "@kentico/xperience-calendar-component.js",
Copy link
Contributor

@michalJakubis michalJakubis Jul 17, 2024

Choose a reason for hiding this comment

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

The file name should be just xperience-calendar-component.js. This is how we name the package file names in XbK. We omit the kentico prefix.

Label = "Date Format",
DefaultValue = "m.d.Y",
ExplanationText = "Select Date format",
Order = 5)]
Copy link
Contributor

Choose a reason for hiding this comment

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

The order parameters seem to be a bit inconsistent. They are not incrementing predictably, and property bellow does not set Order at all. This can lead to an unexpected order of components in final result.

<FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>
<ItemGroup>
<Folder Include="wwwroot\js\" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation in this file is inconsistent.

/// Returns data from dynamic data provider for a given calendar form component.
/// </summary>
/// <param name="dataProviderName">name of the provider</param>
/// <returns></returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty returns description.

@bkapustik bkapustik merged commit 6f619a3 into main Jul 23, 2024
2 checks passed
@michalJakubis michalJakubis deleted the feat/calendarComponent-initial branch July 23, 2024 07:55
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