Enable SrcSet support with two different input formats for Image tag helper #46

Krishanthaudayakumara · 2025-07-09T10:43:32Z

Description / Motivation

This PR implements responsive image support for the ImageTagHelper by adding srcset and sizes attribute functionality, enabling developers to output responsive images using Sitecore's built-in media resizing capabilities.

Key Changes:

Added SrcSet and Sizes Properties: Extended the ImageTagHelper with new properties to support responsive image configuration for both <sc-img> and <img> tags.
Multiple Input Format Support: The SrcSet property accepts two different input formats for maximum flexibility:
- Anonymous objects: new object[] { new { w = 800 }, new { mw = 400 } }
- Dictionary arrays: new Dictionary<string, object> { {"w", 800} }
Content SDK Parameter Priority: Implements the same parameter precedence as Content SDK:
- w (width) takes priority over mw (max width) for srcset descriptors
- Extended support for legacy parameters: w > mw > width > maxWidth
- Only entries with valid width parameters are included in srcset
- Matches Content SDK's getSrcSet function behavior exactly
Enhanced Parameter Merging with Preservation:
- Base imageParams are merged with individual srcSet parameters
- srcSet parameters override imageParams for each srcset entry
- Critical Fix: Added GetSitecoreMediaUriWithPreservation method to preserve existing query parameters
- Preserves essential Sitecore parameters (rev, db, la, vs, ts, hash, ttc, tt, etc.)
- Follows Content SDK's { ...imageParams, ...params } merge pattern
Dual URL Processing Methods:
- Legacy GetSitecoreMediaUri: Maintains existing behavior (strips query parameters)
- New GetSitecoreMediaUriWithPreservation: Preserves critical parameters for responsive images
- GetMediaLinkForSrcSet: Specialized method for srcSet URL generation with parameter preservation
- Ensures backward compatibility while fixing responsive image display issues

Content SDK Compatibility:

The implementation now produces identical output to the Content SDK's Image component:

Content SDK (React):

<Image 
  field={imageField} 
  imageParams={{quality: 80}} 
  srcSet={[{w: 800}, {mw: 400}]} 
  sizes="(min-width: 768px) 800px, 400px" 
/>

ASP.NET Core SDK (Razor):

<sc-img asp-for="@Model.ImageField" 
        image-params='new { quality = 80 }'
        src-set='new object[] { new { w = 800 }, new { mw = 400 } }'
        sizes="(min-width: 768px) 800px, 400px" />

Technical Implementation Details:

Parameter Preservation Architecture:
- MergeParameters(): Merges base and override parameters using reflection and dictionary handling
- GetSitecoreMediaUriWithPreservation(): Preserves existing URL parameters while adding new ones
- ConvertToStringDictionary(): Converts objects to string dictionaries for URL building
- GetMediaLinkForSrcSet(): Specialized extension method for responsive image URL generation
JsonElement Handling: Enhanced support for JSON deserialization scenarios to handle JsonElement types properly
Helper Methods: Added AddResponsiveImageAttributes() overloads to eliminate code duplication across Process(), GenerateImage(), and editable markup methods

Usage Examples:

<!-- Basic responsive image -->
<sc-img asp-for="@Model.HeroImage" 
        src-set='new object[] { new { w = 1200 }, new { w = 800 }, new { w = 400 } }'
        sizes="(min-width: 1200px) 1200px, (min-width: 800px) 800px, 400px" />

<!-- With base parameters (preserves existing URL params) -->
<img asp-for="@Model.ProductImage" 
     image-params='new { quality = 80, format = "webp" }'
     src-set='new object[] { new { w = 800, h = 600 }, new { mw = 400, mh = 300 } }'
     sizes="(min-width: 768px) 800px, 400px" />

<!-- Legacy parameter support -->
<sc-img asp-for="@Model.LegacyImage" 
        src-set='new object[] { new { width = 800 }, new { maxWidth = 400 } }'
        sizes="(min-width: 768px) 800px, 400px" />

This implementation ensures feature parity with the Content SDK while maintaining the familiar ASP.NET Core TagHelper syntax, full backward compatibility, and proper parameter preservation for functional image display.

Testing

The Unit & Integration tests are passing.
I have added the necessary tests to cover my changes.

Terms

I agree to follow this project's Code of Conduct.

Fix #16

…es in ImageTagHelperFixture

…er support, Specific exception handling - Only catch JsonException instead of all exceptions, Better ASP.NET Core compatibility - Replaced HttpUtility with QueryHelpers

…e tests - Keep GetSitecoreMediaUriWithPreservation - Add 14 new test cases covering srcset parameter merging, validation, and edge cases - Fix null handling and zero/negative width filtering in ImageTagHelper - Ensure development environment compatibility with CDN/edge URLs

…ty and comprehensive test cases added Updated - Enable SrcSet support with Content SDK approach compatibility and comprehensive test cases added

Copilot

Pull Request Overview

This PR implements comprehensive responsive image support for the ImageTagHelper by adding srcset and sizes attribute functionality. The implementation enables developers to output responsive images using Sitecore's built-in media resizing capabilities while maintaining compatibility with the Content SDK behavior.

Added SrcSet and Sizes properties to ImageTagHelper with support for multiple input formats (anonymous objects, dictionary arrays, JSON strings)
Implemented parameter merging logic that preserves existing URL parameters while allowing srcSet parameters to override base imageParams
Added extensive test coverage for various srcSet scenarios including parameter precedence, validation, and edge cases

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`ImageTagHelper.cs`	Core implementation of srcSet/sizes support with parameter parsing and URL generation logic
`SitecoreFieldExtensions.cs`	Added parameter preservation methods and URL building logic for responsive images
`ImageTagHelperFixture.cs`	Comprehensive test suite covering all srcSet functionality and edge cases

src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

…ng StringComparer.OrdinalIgnoreCase for the dictionary to ensure consistent parameter key handling

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs

tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs

…directly creating the final mergedParams dictionary

…ethod - ApplyJssMediaUrlPrefix

…enderingEngine

…ue] return

…t to ParseJsonSrcSet, move the type checks to the calling method and simplifying ParseJsonSrcSet

…he test cases to validate the fix

robearlam · 2025-08-01T00:49:41Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+        string urlWithoutQuery;
+        string? query = null;
+
+        if (uri.IsAbsoluteUri)
+        {
+            urlWithoutQuery = uri.GetLeftPart(UriPartial.Path);
+            query = uri.Query;
+        }
+        else
+        {
+            // For relative URIs, manually split on '?'
+            var original = uri.OriginalString;
+            int idx = original.IndexOf('?');
+            if (idx >= 0)
+            {
+                urlWithoutQuery = original.Substring(0, idx);
+                query = original.Substring(idx);
+            }
+            else
+            {
+                urlWithoutQuery = original;
+            }
+        }
+
+        if (!string.IsNullOrEmpty(query))
+        {
+            var parsedQuery = QueryHelpers.ParseQuery(query);
+            foreach (KeyValuePair<string, StringValues> kvp in parsedQuery)
+            {
+                parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null;
+            }
+        }
+
+        return urlWithoutQuery;


It feels like we could simplify this massively. Would the following work instead?

string url = $"${uri.Scheme}://${uri.Host}"; NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query); foreach (string? param in queryParams.AllKeys) { if (!string.IsNullOrEmpty(param)) { parameters[param] = queryParams[param]; } } return url;

Thanks for the suggestion Rob, I tested with this but couple of test cases get failed, since it requires to handle relative URL's as well. So I tried to simplified it in this way -

string original = uri.OriginalString; int queryIndex = original.IndexOf('?'); if (queryIndex >= 0) { string query = original.Substring(queryIndex); var parsedQuery = QueryHelpers.ParseQuery(query); foreach (var kvp in parsedQuery) { parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } return original.Substring(0, queryIndex); } return original;

b15b4c1

You shouldn't need to do all of that string matching, that's what I'm trying to avoid. You should be able to get all of the information you need from the Uri class. Check out the docs here so you can see what I mean: https://learn.microsoft.com/en-us/dotnet/api/system.uri?view=net-8.0

Maybe something like:

string url = uri.IsAbsoluteUri ? $"${uri.Scheme}://{uri.Host}{uri.AbsolutePath}" : uri.AbsolutePath; NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query); foreach (string? param in queryParams.AllKeys) { if (!string.IsNullOrEmpty(param)) { parameters[param] = queryParams[param]; } } return url;

Hi Rob,
Thanks for clarifying, I've tried and tested the approach you suggested in couple of different ways, using only the Uri class properties (like uri.AbsolutePath and uri.Query) to avoid manual string matching, as described in the .NET docs. This works for absolute URIs and root relative URIs (e.g., /media/image.png). However, in .NET, if I pass a simple relative URI such as media/image.png?foo=1, accessing uri.Query will throw an InvalidOperationException. That causes some of the tests get failed. Let me know what's your suggestions on this.

Update : I've updated the ParseUrlParams method to handle absolute and relative URIs separately.

For absolute URIs, I'm using Uri properties (Scheme, Host, AbsolutePath, Query) and HttpUtility.ParseQueryString.

For relative URIs, since accessing Uri.Query throws InvalidOperationException, I'm falling back to string matching and using QueryHelpers.ParseQuery.

ab6e358

…ions to handle absolute and relative URIs separately

… absolute URIs separately - Replace string manipulation with Uri class properties for absolute URIs - Add proper handling for relative URIs that throw InvalidOperationException when accessing Uri.Query - Use HttpUtility.ParseQueryString for absolute URIs and QueryHelpers.ParseQuery for relative URIs - Add explanatory comments for the if/else branching logic - Simplify ParseUrlParams method while maintaining compatibility with all URI types

sc-ivanlieckens · 2025-08-18T07:15:49Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+
+        if (urlStr == null)
+        {
+            return null;


Please avoid multiple return statements.

Thanks for the feedbacks Ivan, I refactored GetMediaLink to use a single return statement - Krishanthaudayakumara#26

Same adjustment to GetMediaLinkForSrcSet?

Sure Ivan, Did the same for GetMediaLinkForSrcSet here - https://github.com/Krishanthaudayakumara/ASP.NET-Core-SDK/pull/30/files

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

sc-ivanlieckens · 2025-08-18T07:32:36Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+    {
+        if (parameters == null)
+        {
+            return;


I understand the "escape hatch" here for performance reasons; but I hate return statements like these. My IDE turns this method into the following:

private static void AddParametersToResult(Dictionary<string, object?> result, object? parameters, bool skipNullValues = false) { switch (parameters) { case null: break; case Dictionary<string, object?> paramDict: foreach (KeyValuePair<string, object?> kvp in paramDict.Where(kvp => !skipNullValues || kvp.Value != null)) { result[kvp.Key] = kvp.Value; } break; default: RouteValueDictionary routeValues = new(parameters); foreach (KeyValuePair<string, object?> kvp in routeValues.Where(kvp => !skipNullValues || kvp.Value != null)) { result[kvp.Key] = kvp.Value; } break; } }

Same result, different readability. Also read into the constructor of RouteValueDictionary, it'll show you a cool method of extracting values from a variety of valid parameter formats without locking yourself onto a specific one. Because seems we don't have overrides for Dictionary<string, object?> specifically anywhere making it less likely for that code branch to be used?

I get the point Ivan, So refactored AddParametersToResult to use a switch pattern, eliminating the early return and improving readability - Krishanthaudayakumara#26

And what did you think of the RouteValueDictionary constructor?

I guess the RouteValueDictionary constructor is a handy escape hatch , it accepts IDictionary, POCOs and anonymous objects , normalises keys and copies values so we don’t need lots of special cases.

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs

…ng the explicit type declarations with target-typed new expressions

…LINQ to eliminate the early return statement

…atement in the ParseUrlParams method

…pers

…tAttribute - ImageTagHelper

…8 - Style, Readability, and Improvements - Refactored methods to avoid multiple return statements and redundant branches. - Replaced explicit type declarations with target-typed new expressions (IDE0090). - Eliminated early returns in favor of switch/case patterns for better readability. - Updated substring extraction and return statements to use C# range indexers. - Removed all var usages, replacing them with explicit types. - Removed unnecessary null checks and used collection initializer syntax for lists. - Improved overall code clarity, consistency, and compliance with project style guidelines.

sc-ivanlieckens · 2025-09-05T07:14:19Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+
+        if (urlStr == null)
+        {
+            return null;


Same adjustment to GetMediaLinkForSrcSet?

sc-ivanlieckens · 2025-09-05T07:22:56Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+    /// <param name="uri">The Uri with potential query parameters.</param>
+    /// <param name="parameters">The dictionary to add parsed parameters to.</param>
+    /// <returns>The URL without query parameters.</returns>
+    private static string ParseUrlParams(Uri? uri, Dictionary<string, object?> parameters)


Did we squeeze 3 functions into 1? Seeing all these returns I would consider perhaps extracting them as a function on their own and isolating those returns allowing you to do the "branch selection" in this function. As I mentioned earlier in this same file, I dislike multiple return statements as it makes debugging harder.

Make sense Ivan, I refactored URL parsing to single return style with extracting ParseAbsoluteUriParams and ParseRelativeUriParams as separate methods - https://github.com/Krishanthaudayakumara/ASP.NET-Core-SDK/pull/30/files

sc-ivanlieckens · 2025-09-05T07:32:30Z

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

+    {
+        if (parameters == null)
+        {
+            return;


And what did you think of the RouteValueDictionary constructor?

- Extract ParseAbsoluteUriParams and ParseRelativeUriParams; make ParseUrlParams a single-return wrapper. - Convert ParseRelativeUriParams and GetSitecoreMediaUriWithPreservation to single-return style. - Preserve existing parsing behavior for absolute and relative URIs.

…ingle return

…ediaUriWithPreservation

…le - PR Review - 09/15 - Extract ParseAbsoluteUriParams and ParseRelativeUriParams; make ParseUrlParams a single-return wrapper. - Convert ParseRelativeUriParams and GetSitecoreMediaUriWithPreservation to single-return style. - Preserve existing parsing behavior for absolute and relative URIs. - Improve debuggability and reduce branching/early return

Krishanthaudayakumara added 2 commits July 9, 2025 11:43

Enable SrcSet support with three different input formats and test cas…

2a8f19c

…es in ImageTagHelperFixture

Remove unnecessary white spaces remained in changes

2d719cd

robearlam requested review from Copilot and robearlam and removed request for robearlam July 9, 2025 22:56

This comment was marked as outdated.

Sign in to view

robearlam marked this pull request as draft July 11, 2025 00:48

Krishanthaudayakumara added 7 commits July 14, 2025 14:09

Add changes to match with content SDK

0937e2f

Alternative approach for parameter preservation in srcSet functionality

defd382

Enhanced parameter support - Added legacy width and maxWidth paramet…

6fc8c4f

…er support, Specific exception handling - Only catch JsonException instead of all exceptions, Better ASP.NET Core compatibility - Replaced HttpUtility with QueryHelpers

Replace var keywords with explicit type declarations

eaa8bc5

Remove unnessasary GetMediaLink method overloading

454e049

Updated - Enable SrcSet support with Content SDK approach compatibili…

50c37ba

…ty and comprehensive test cases added Updated - Enable SrcSet support with Content SDK approach compatibility and comprehensive test cases added

Krishanthaudayakumara requested a review from Copilot July 16, 2025 10:16

Copilot AI reviewed Jul 16, 2025

View reviewed changes

Add comment explanation of GetMediaLinkForSrcSet preservation and usi…

43a09f8

…ng StringComparer.OrdinalIgnoreCase for the dictionary to ensure consistent parameter key handling

robearlam marked this pull request as ready for review July 17, 2025 00:43

robearlam requested changes Jul 17, 2025

View reviewed changes

Krishanthaudayakumara added 10 commits July 17, 2025 09:04

refactor the code to eliminate the unnecessary dictionary copying by …

10c272a

…directly creating the final mergedParams dictionary

extract the duplicated URL prefix replacement logic into a separate m…

21d8af3

…ethod - ApplyJssMediaUrlPrefix

reorder the methods below the properties in ImageTagHelper.cs - SDK.R…

87fabe5

…enderingEngine

updated ParseSrcSet in - ImageTagHelper.cs code to use the [srcSetVal…

8b1bff5

…ue] return

Remove unnessasary comments in ImageTagHelper and refactor ParseSrcSe…

3e0cb92

…t to ParseJsonSrcSet, move the type checks to the calling method and simplifying ParseJsonSrcSet

Remove unnessasary comments in ImageTagHelper and refactor ParseSrcSe…

99aa164

…t to ParseJsonSrcSet, move the type checks to the calling method and simplifying ParseJsonSrcSet

Throw exception in invalid JSON passing in ParseJsonSrcSet, updated t…

ac23fcb

…he test cases to validate the fix

Add clarification comment for the use of Dictionary<string, object>[]

f87a037

optimize this by using LINQ's Where() - in GenerateSrcSetAttribute

edd2bfd

Remove Unnecessary comment

efe5b45

Krishanthaudayakumara changed the title ~~Enable SrcSet support with three different input formats for Image tag helper~~ Enable SrcSet support with two different input formats for Image tag helper Jul 31, 2025

Krishanthaudayakumara requested a review from robearlam July 31, 2025 05:21

robearlam requested changes Aug 1, 2025

View reviewed changes

Simplify ParseUrlParams to handle relative URIs without exceptions

b15b4c1

Krishanthaudayakumara requested a review from robearlam August 1, 2025 04:50

Krishanthaudayakumara added 2 commits August 5, 2025 14:07

refactor: ParseUrlParams - improve URI parsing in SitecoreFieldExtens…

b3e5a58

…ions to handle absolute and relative URIs separately

sc-ivanlieckens requested changes Aug 18, 2025

View reviewed changes

sc-ivanlieckens added the enhancement New feature or request label Aug 18, 2025

sc-ivanlieckens added this to ASP.NET Core SDK Aug 18, 2025

sc-ivanlieckens moved this to In review in ASP.NET Core SDK Aug 18, 2025

Krishanthaudayakumara added 11 commits August 19, 2025 10:37

Avoid multiple return statements and return null in GetMediaLink

03285bf

fix IDE0090 violations in the SitecoreFieldExtensions file by replaci…

d227223

…ng the explicit type declarations with target-typed new expressions

Refactor AddParametersToResult using a switch expression pattern and …

d87bdfc

…LINQ to eliminate the early return statement

set blank line after closing brace of following change

7916141

remove the redundant else block and consolidate to a single return st…

47c09bf

…atement in the ParseUrlParams method

uses the range indexer original[queryIndex..] in parseUrlParams

6b64d7a

Remove var - use explicit types instead of var in changes

8b709c5

use original[..queryIndex]; in ParseUrlParams

c5e6a17

Remove unnessasary if branch in GenerateSrcSetAttribute - ImageTagHel…

e0652bc

…pers

use collection initializer syntax to create the list in GenerateSrcSe…

50f7051

…tAttribute - ImageTagHelper

Krishanthaudayakumara requested a review from sc-ivanlieckens August 19, 2025 09:51

sc-ivanlieckens requested changes Sep 5, 2025

View reviewed changes

Krishanthaudayakumara added 5 commits September 8, 2025 10:53

Remove multiple return statements - GetMediaLinkForSrcSet

40e565b

Keep null semantics and the explicit IsNullOrEmpty guard, and use a s…

be01987

…ingle return

Ensure we only drop the path when URI parsing succeedsin GetSitecoreM…

3184370

…ediaUriWithPreservation

Krishanthaudayakumara requested a review from sc-ivanlieckens September 8, 2025 09:24

Enable SrcSet support with two different input formats for Image tag helper #46

Are you sure you want to change the base?

Enable SrcSet support with two different input formats for Image tag helper #46

Uh oh!

Conversation

Krishanthaudayakumara commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description / Motivation

Key Changes:

Content SDK Compatibility:

Technical Implementation Details:

Usage Examples:

Testing

Terms

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Krishanthaudayakumara Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Krishanthaudayakumara Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Krishanthaudayakumara Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Krishanthaudayakumara commented Jul 9, 2025 •

edited

Loading

Krishanthaudayakumara Aug 1, 2025 •

edited

Loading

Krishanthaudayakumara Aug 19, 2025 •

edited

Loading

Krishanthaudayakumara Sep 8, 2025 •

edited

Loading