Skip to content

fix: date range resources - extend calendar & extra props - #295

Open
damyanpetev wants to merge 2 commits into
masterfrom
dpetev/date-picker-resource-strings
Open

fix: date range resources - extend calendar & extra props#295
damyanpetev wants to merge 2 commits into
masterfrom
dpetev/date-picker-resource-strings

Conversation

@damyanpetev

Copy link
Copy Markdown
Member

Description

Discovered from #286 - finding was the Date Range Picker resource strings are empty, but they should also extend the calendar ones, so fixed both and updated docs.

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog

Component(s) / Area(s) Affected:

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

Test Configuration:

  • .NET version:
  • Hosting model:
  • Browser(s):
  • OS:

Screenshots / Recordings

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code
  • This PR includes API docs for newly added methods/properties
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes behavioral changes and the feature specification has been updated with them
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

Closes #

@damyanpetev
damyanpetev requested a review from skrustev August 4, 2026 07:41
Copilot AI review requested due to automatic review settings August 4, 2026 07:41
@damyanpetev damyanpetev added 📖 documentation Improvements or additions to documentation 🐛 bug Something isn't working ❌ status: awaiting-test PRs awaiting manual verification 📅 date-range-picker labels Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes the Date Range Picker localization surface by ensuring its resource strings are no longer empty and properly inherit the Calendar resource strings, aligning the Blazor wrapper with the upstream web component contract referenced in #286.

Changes:

  • Add Date Range Picker–specific resource string parameters (Separator/DoneButton/CancelButton) to the Blazor wrapper and serialization.
  • Extend Date Range Picker resource string metadata to include the Calendar resource string keys.
  • Update the DateRangePicker contract test to validate non-empty resource string wiring.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/IgniteUI.Blazor.Tests/DateRangePickerTests.cs Updates the contract test to assert resource strings serialize with expected keys/values.
src/src/ig/igniteui-core/WebDateRangePickerResourceStringsDescriptionMetadata.ts Attempts to extend DateRangePicker resource-string metadata with calendar metadata and add new keys.
src/src/ig/igniteui-core/WebDateRangePickerResourceStringsDescription.ts Adds TS description properties for separator/done/cancel resource strings.
src/components/Blazor/DateRangePickerResourceStrings.cs Makes Blazor resource strings inherit calendar strings and adds new parameters + serialization.
Suppressed comments (1)

src/src/ig/igniteui-core/WebDateRangePickerResourceStringsDescriptionMetadata.ts:34

  • fillMetadata/register are delegating to WebCalendarDescriptionMetadata, which will register calendar component metadata and pollute the resource-strings metadata dictionary. Delegate to WebCalendarResourceStringsDescriptionMetadata instead so DateRangePicker resource strings correctly inherit the calendar resource-string keys (and __isPlainObject).
	static fillMetadata(metadata: Dictionary$2<string, string>): void {
		WebCalendarDescriptionMetadata.fillMetadata(metadata);
		metadata.item("__qualifiedNameTS", "String:DateRangePickerResourceStrings");
		metadata.item("__skipModuleRegisterWebComponents", "Boolean");
		metadata.item("__importTypesWebComponents", "String:igniteui-webcomponents");
		metadata.item("Separator", "String");
		metadata.item("DoneButton", "(wc:Done)String");
		metadata.item("CancelButton", "(wc:Cancel)String");
	}
	static register(context: TypeDescriptionContext): void {
		WebCalendarDescriptionMetadata.register(context);
		WebDateRangePickerResourceStringsDescriptionMetadata.ensureMetadata(context);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/components/Blazor/DateRangePickerResourceStrings.cs:44

  • For consistency with the rest of the wrapper codegen, add the missing OnDoneButtonChanging hook for DoneButton. Also, other WCWidgetMemberName usages in the codebase use PascalCase (e.g. WCWidgetMemberName("Name")), so use "Done" here as well.
        private string _doneButton;
        /// <summary>
        /// Text for the picker button that commits the range selection.
        /// </summary>
        [Parameter]

src/components/Blazor/DateRangePickerResourceStrings.cs:64

  • For consistency with the rest of the wrapper codegen, add the missing OnCancelButtonChanging hook for CancelButton. Also, other WCWidgetMemberName usages in the codebase use PascalCase, so use "Cancel" here as well.
        private string _cancelButton;
        /// <summary>
        /// Text for the picker button that cancels the range selection.
        /// </summary>
        [Parameter]

src/components/Blazor/DateRangePickerResourceStrings.cs:26

  • For consistency with other generated wrappers (e.g. CalendarResourceStrings.cs declares a partial void On<Property>Changing(ref string newValue); for each parameter), add the missing OnSeparatorChanging hook before the Separator parameter.

This issue also appears in the following locations of the same file:

  • line 40
  • line 60
        private string _separator;
        /// <summary>
        /// The text shown between the start and end inputs when the date range picker is configured with separate inputs.
        /// </summary>
        [Parameter]


private string _doneButton;
/// <summary>
/// Text for the picker button that commits the range selection.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe add "in dialog mode"?


private string _cancelButton;
/// <summary>
/// Text for the picker button that cancels the range selection.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also only in dialog mode.


}
}

@skrustev skrustev Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing last7Days, last30Days, currentMonth, yearToDate or we can defer that for when the localization refactor is done?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, missed to leave that in the description - intentionally added only the ones I found in use, so I can avoid the awkward "what does this option do and what to do with it" problem we have with the calendar ones :)

@skrustev skrustev Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The ones I listed are also used, just have to be enabled through the UsePredefinedRanges property I think also for dialog mode:

image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, those weren't used though resourceStrings. so I didn't see them! Should be checking indexed access too I guess :). Okay, will add them as well

@damyanpetev damyanpetev added the squash-merge Merge PR with "Squash and Merge" option label Aug 4, 2026
@damyanpetev
damyanpetev requested a review from skrustev August 4, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📖 documentation Improvements or additions to documentation 🐛 bug Something isn't working 📅 date-range-picker squash-merge Merge PR with "Squash and Merge" option ❌ status: awaiting-test PRs awaiting manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants