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 Internet Explorer 11 specific logic from KTextTruncator #651

Closed
wants to merge 1 commit into from
Closed

Conversation

Nivas7
Copy link
Contributor

@Nivas7 Nivas7 commented Jun 5, 2024

Remove Internet Explorer 11 specific logic from KTextTruncator - #643

Description

In summary, to remove IE11 support from the KTextTruncator component:

Remove the lineHeightIE prop.
Remove the IE11 fallback logic and related styles from the truncate method.

This change, treated as a breaking change, aims to simplify the component by removing unnecessary code for the unsupported IE11 browser. The updated component should be thoroughly tested to ensure no regressions.

Addresses #PR# HERE

Before/after screenshots

Before
image

After
image

Changelog

  • [Remove Internet Explorer 11 specific logic from KTextTruncator #643]
    • Description: Summary of change(s)
    • Products impact: Choose from - none (for internal updates) / bugfix / new API / updated API / removed API. If it's 'none', use "-" for all items below to indicate they are not relevant.
    • Addresses: Link(s) to GH issue(s) addressed. Include KDS links as well as links to related issues in a consumer product repository too.
    • Components: Affected public KDS component. Do not include internal sub-components or documentation components.
    • Breaking: Will this change break something in a consumer? Choose from: yes / no
    • Impacts a11y: Does this change improve a11y or adds new features that can be used to improve it? Choose from: yes / no
    • Guidance: Why and how to introduce this update to a consumer? Required for breaking changes, appreciated for changes with a11y impact, and welcomed for non-breaking changes when relevant.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included

Reviewer guidance

  • Is the code clean and well-commented?

After review

  • The changelog item has been pasted to the CHANGELOG.md

@Nivas7
Copy link
Contributor Author

Nivas7 commented Jun 5, 2024

I hope this Problem Will be solved?

@MisRob MisRob self-requested a review June 5, 2024 17:19
@MisRob MisRob self-assigned this Jun 5, 2024
@@ -1,19 +1,4 @@
<template>

<!--
Copy link
Member

Choose a reason for hiding this comment

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

It's fine to cleanup comments that are specific to IE11, but please make sure to read them carefully and do not remove comments that have nothing to do with IE11 logic. This applies to all other removed comments in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's Good Catch!

Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Hey @Nivas7, thanks! Please see my note about comments. Resolving it should also help with missing "Props" descriptions for text and maxLines (can be seen in the "After" screenshots)

@MisRob
Copy link
Member

MisRob commented Jun 5, 2024

Also please run linter

@Nivas7
Copy link
Contributor Author

Nivas7 commented Jun 5, 2024

I will make that changes quickly:)

@Nivas7 Nivas7 closed this by deleting the head repository Jun 6, 2024
@Nivas7
Copy link
Contributor Author

Nivas7 commented Jun 6, 2024

apologize for the mistake i re fork my repo and made my pr

@Nivas7
Copy link
Contributor Author

Nivas7 commented Jun 6, 2024

How can i made pull req to develop barnch from fork it has only releases channellog.md not develop branch challange.md

@Nivas7
Copy link
Contributor Author

Nivas7 commented Jun 6, 2024

I solved the linting error and removed the Internet explorer props and fallback function i got so much confusion in pushing this to develop Branch , I Apologized for this

@MisRob
Copy link
Member

MisRob commented Jun 7, 2024

Hi @Nivas7, no problem. I'd recommend you pull the latest develop from upstream to your local, start a new branch from it, and then open a new pull request. It may be helpful to spend a while looking into some tutorials on git. If you don't see the upstream's develop on your forked repository, this could help https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo#configuring-git-to-sync-your-fork-with-the-upstream-repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants