Skip to content
This repository was archived by the owner on Sep 18, 2025. It is now read-only.

Replace URLs with ASP attributes where necessary. Add XML documentation. Minor corrections.#53

Merged
akdalin-hee merged 1 commit into
mainfrom
feature/TD-5572-Replace-urls-with-asp-attributes
Jun 3, 2025
Merged

Replace URLs with ASP attributes where necessary. Add XML documentation. Minor corrections.#53
akdalin-hee merged 1 commit into
mainfrom
feature/TD-5572-Replace-urls-with-asp-attributes

Conversation

@akdalin-hee
Copy link
Copy Markdown

@akdalin-hee akdalin-hee commented May 8, 2025

Description

Replace URLs with ASP attributes where necessary. Add XML documentation where missing and remove duplicate documentation from ViewComponents relative to their ViewModel. Minor corrections.

Screenshots

Paste screenshots for all views created or changed: mobile, tablet and desktop, wave analyser showing no errors.


Developer checks

(Leave tasks unticked if they haven't been appropriate for your ticket.)

I have:

  • Run the IDE auto formatter on all files I’ve worked on and made sure there are no IDE errors relating to them
  • Written or updated tests for the changes and made sure all tests are passing
  • Manually tested my work with and without JavaScript (adding notes where functionality requires JavaScript)
  • Tested with Wave Chrome plugin. Addressed any valid accessibility issues and documented any invalid errors
  • Scanned over my pull request in GitHub and addressed any warnings from the GitHub Build and Test checks in the GitHub PR ‘Files Changed’ tab
    Either:
  • Documented my work in Confluence, updating any business rules applied or modified. Updated GitHub readme/documentation for the repository if appropriate. List of documentation links added/changed:
  • Confirmed that none of the work that I have undertaken requires any updates to documentation
  • CHANGELOG entry

Copy link
Copy Markdown

@kevwhitt-hee kevwhitt-hee left a comment

Choose a reason for hiding this comment

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

These look good to me, Dalin. We seem to have removed some documentation as well as adding some. Was this deliberate? Can we add a note to the PR description explaining why, if so, please?

/// <param name="aspAction">The asp-action to dynamically set the action path of the link..</param>
/// <param name="aspRouteData">The asp-all-route-data to dynamically set the extra parameters to the path of the link.</param>
/// <param name="url">The url for the link.</param>
/// <param name="style">The style attributes for the button.</param><param name="style">The style attributes for the button.</param>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have we deliberately removed these, Dalin?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, it was in order to remove duplication in the XML documentation.

Copy link
Copy Markdown

@kevwhitt-hee kevwhitt-hee left a comment

Choose a reason for hiding this comment

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

Looks good.

@akdalin-hee akdalin-hee merged commit f35bc7a into main Jun 3, 2025
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants