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

Rename summary component parameters #3224

Conversation

precious-onyenaucheya-ons
Copy link
Contributor

@precious-onyenaucheya-ons precious-onyenaucheya-ons commented May 30, 2024

What is the context of this PR?

BREAKING CHANGE
Fixes #3229

In this PR, we are addressing an inconsistency in the naming of parameters within the summary component. This change aims to standardize the parameter names for clarity and consistency.

The following parameter renames are included:

  • summary.summaryTitle is renamed to summary.title
  • group.groupTitle is renamed to group.title
  • row.rowTitle is renamed to row.title
  • row.rowItems is renamed to row.items
  • rowItem.rowTitle is renamed to item.title
  • rowItem.rowTitleAttributes is renamed to item.titleAttributes

This change is a breaking change because it alters the existing parameter names, which will require updates wherever these parameters are used .

How to Review this PR

  1. Ensure all instances of the renamed parameters have been updated throughout the codebase.
  2. Verify that the summary component works correctly after these changes.
  3. Run all tests to ensure no functionality is broken by this update.

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@precious-onyenaucheya-ons precious-onyenaucheya-ons added Do not merge Don't merge this PR until this label is removed Breaking changes This PR contains at least one breaking change labels May 30, 2024
@precious-onyenaucheya-ons precious-onyenaucheya-ons requested a review from a team May 30, 2024 17:32
Copy link

netlify bot commented May 30, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit d29e11e
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/66752b5b54e192000965b68e
😎 Deploy Preview https://deploy-preview-3224--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@precious-onyenaucheya-ons precious-onyenaucheya-ons added Tech improvements Tech debt, cleanup, code standardisation etc. and removed Breaking changes This PR contains at least one breaking change labels May 30, 2024
Copy link
Contributor

@alessioventuriniAND alessioventuriniAND left a comment

Choose a reason for hiding this comment

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

Should we also replace rowItems with just items?

@rmccar rmccar changed the title rename confusing parameters in summary component Rename confusing parameters in summary component Jun 3, 2024
@rmccar rmccar changed the title Rename confusing parameters in summary component Rename summary component parameters Jun 3, 2024
@alessioventuriniAND alessioventuriniAND added the Breaking changes This PR contains at least one breaking change label Jun 4, 2024
@precious-onyenaucheya-ons precious-onyenaucheya-ons force-pushed the feature/1730-rename-params-in-summary-component branch from 6756888 to e21fa68 Compare June 4, 2024 10:02
@precious-onyenaucheya-ons
Copy link
Contributor Author

Should we also replace rowItems with just items?

updated

Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

I think there are some references in the summary tests that also need updating

@alessioventuriniAND alessioventuriniAND changed the base branch from main to major-release September 3, 2024 09:48
@alessioventuriniAND alessioventuriniAND merged commit aa9f5cd into major-release Sep 6, 2024
8 of 9 checks passed
@alessioventuriniAND alessioventuriniAND deleted the feature/1730-rename-params-in-summary-component branch September 6, 2024 15:27
@alessioventuriniAND alessioventuriniAND removed the Do not merge Don't merge this PR until this label is removed label Sep 6, 2024
This was referenced Sep 10, 2024
@rmccar rmccar changed the title Rename summary component parameters Rename summary component parameters Sep 13, 2024
@alessioventuriniAND alessioventuriniAND mentioned this pull request Sep 13, 2024
2 tasks
rmccar pushed a commit that referenced this pull request Oct 9, 2024
rmccar pushed a commit that referenced this pull request Oct 9, 2024
rmccar pushed a commit that referenced this pull request Oct 9, 2024
rmccar pushed a commit that referenced this pull request Oct 9, 2024
* Standardise setting heading level across components #3111

* Update param names to camel case naming standard #3188

* Renamed utility class from ons-grid--flex to ons-grid-flex #3268

* Removed metadata component #3275

* Remove call to action component and all references #3143

* Update parameter from UnreadLink to UnreadLinkUrl in Message component #3290

* Update parameter subject from text to object type in Message list component #3291

* Add option for additional logo in footer #3227

* Update parameter from link to text in Footer Component #3294

* Update parameter name from url to src in Image Component #3293

* Update parameter names from LogoImage and LogoURL to image and url a in header component #3292

* Update parameter name from url to linkUrl in feedback component #3295

* Rename summary component parameters #3224

* Refactor timeline macro for compatibility with Jinja2 templates #3180

* Update parameter names in document list component #3299

* Update param name settingsLinkTextURL to settingsLinkURL in Cookies Banner component #3300

* Update parameter names in card component #3303

* Update parameter from linkText to text in external link component #3304

* Update param name from title to text in section navigation component #3305

* Update parameter from linkText to videoLinkText in Video Component #3306

* Update parameter from manualLink to manualLinkUrl in Address Input component #3307

* Ensure list item custom attributes are always set on list item #3336

* Rename items summary param for compatibility with Jinja #3382

* Update DS components to ensure compatibility with Jinja #3380

* Rename html OGL Link param to current naming standard #3379

* Format migration guide #3377

* Introduction of new spacing grid #3285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking changes This PR contains at least one breaking change Tech improvements Tech debt, cleanup, code standardisation etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename summary component parameters
4 participants