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

Remove duplicate class on related-content example #3262

Conversation

precious-onyenaucheya-ons
Copy link
Contributor

@precious-onyenaucheya-ons precious-onyenaucheya-ons commented Jul 10, 2024

What is the context of this PR?

Fixes #3236
The related links social example had the ons-list--bare class added twice. This PR updates the logic for the related content to ensure that the right class is added. Additionally, the documentation has been updated to include variants as a parameter.

How to review this PR

Verify that the example in question no longer has the duplicate class added.
Ensure all tests pass.
Confirm that the documentation has been updated accordingly.

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 the Bug Something isn't working label Jul 10, 2024
@precious-onyenaucheya-ons precious-onyenaucheya-ons requested a review from a team July 10, 2024 14:05
Copy link

netlify bot commented Jul 10, 2024

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

Name Link
🔨 Latest commit ffc98bf
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/669a5de14a46c200084e4ff6
😎 Deploy Preview https://deploy-preview-3262--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 changed the title update logic in related-content macro remove duplicate class on related-content example Jul 11, 2024
@rmccar rmccar changed the title remove duplicate class on related-content example Remove duplicate class on related-content example Jul 11, 2024
@precious-onyenaucheya-ons precious-onyenaucheya-ons force-pushed the fix/3236-duplicate-class-on-related-links-social-example branch from 5bdfa0d to 2b94422 Compare July 11, 2024 20:50
@rmccar
Copy link
Contributor

rmccar commented Jul 19, 2024

Because this has changed quite a bit and Im the only one that has reviewed it since the changes Ive requested some re-reviews

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.

Can you please update the body of the PR to also include the change you have made around incorporating the social style in the native behaviour of the component and remove the social variant?

@precious-onyenaucheya-ons precious-onyenaucheya-ons merged commit b1570ab into main Jul 22, 2024
9 checks passed
@precious-onyenaucheya-ons precious-onyenaucheya-ons deleted the fix/3236-duplicate-class-on-related-links-social-example branch July 22, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate class on related links social example
5 participants