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

Refactor timeline macro for compatibility with Jinja2 templates #3180

Merged

Conversation

SriHV
Copy link
Contributor

@SriHV SriHV commented May 8, 2024

What is the context of this PR?

#3171 ...

BREAKING CHANGE

We have made a breaking change to the timeline macro by renaming the items parameter to timelineItems. This change affects all instances where the timeline macro is used. Below, we provide detailed instructions on how to update your service to accommodate this change.

How to Update Your Code
To update your service, you need to replace the items parameter with timelineItems wherever the timeline macro is used. Here is an example of how to make this change:

Before:
{{ onsTimeline({ "items": [. . . ] }) }}

After:
{{ onsTimeline({ "timelineItems": [. . . ] }) }}

How to review this PR

  • Test the changes in Design system
  • Pull the latest PR (link) which is in review. Follow the updated readme.
  • Update the macro and test it in Flask.

Checklist

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

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

@SriHV SriHV self-assigned this May 8, 2024
Copy link

netlify bot commented May 8, 2024

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

Name Link
🔨 Latest commit ce199e1
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/668e977473729200081873a4
😎 Deploy Preview https://deploy-preview-3180--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.

@SriHV SriHV added the Bug Something isn't working label May 8, 2024
@SriHV SriHV linked an issue May 8, 2024 that may be closed by this pull request
@SriHV SriHV requested a review from a team May 8, 2024 09:51
@SriHV SriHV marked this pull request as ready for review May 8, 2024 09:51
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.

I am not sure I understand what has been changed? Looks like a parameter name has been changed from items to items_timeline but I didn't see any .items() functions?

@SriHV
Copy link
Contributor Author

SriHV commented May 8, 2024

I am not sure I understand what has been changed? Looks like a parameter name has been changed from items to items_timeline but I didn't see any .items() functions?

oh sorry I wasn't clear. Items is a reserved name mainly used as a built-in function for dictionary data structure. So when trying to iterate over this parameter named 'items', we get this error 'builtin_function_or_method' object is not iterable.

@rmccar
Copy link
Contributor

rmccar commented May 9, 2024

I am not sure I understand what has been changed? Looks like a parameter name has been changed from items to items_timeline but I didn't see any .items() functions?

oh sorry I wasn't clear. Items is a reserved name mainly used as a built-in function for dictionary data structure. So when trying to iterate over this parameter named 'items', we get this error 'builtin_function_or_method' object is not iterable.

Can this information be put into the description

@rmccar rmccar changed the title Refactor timeline macro to suit Jinja2 templates Refactor timeline macro for compatibility with Jinja2 templates May 9, 2024
src/components/timeline/_macro-options.md Outdated Show resolved Hide resolved
@rmccar rmccar added Breaking changes This PR contains at least one breaking change Do not merge Don't merge this PR until this label is removed and removed Bug Something isn't working labels May 9, 2024
@rmccar
Copy link
Contributor

rmccar commented May 9, 2024

Can we have a "breaking changes" section added to the description that tells the user how to update their service to fix the breaking change we have introduced

@alessioventuriniAND alessioventuriniAND mentioned this pull request May 9, 2024
7 tasks
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.

As richard said can you please add a Breaking change section to explain how users can fix this in their service?

@alessioventuriniAND alessioventuriniAND changed the base branch from main to major-release September 3, 2024 09:49
@alessioventuriniAND alessioventuriniAND merged commit bdd2afb into major-release Sep 6, 2024
8 of 10 checks passed
@alessioventuriniAND alessioventuriniAND deleted the fix/3171/refactor-timeline-to-suit-Jinja2 branch September 6, 2024 15:37
@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 added a commit that referenced this pull request Oct 9, 2024
* Removed .push method

* Changed items name

* Removing conflicts

* changes as per comments

* changes as per comments

* Changing indentation

* Update src/components/timeline/example-timeline.njk

Co-authored-by: rmccar <[email protected]>

---------

Co-authored-by: rmccar <[email protected]>
Co-authored-by: Alessio Venturini <[email protected]>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor timeline macro to suit Jinja2 templates
4 participants