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

fix: [M3-8696] - Autocomplete renderOption prop console warning #11140

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Oct 22, 2024

Description 📝

Fixes this annoying little bugger when overriding renderOption in AutoComplete.

Screenshot 2024-10-22 at 15 05 09

It was already fixed when not overriding in https://github.com/linode/manager/pull/11041/files, alas we needed a fix when passing renderOption. I did not find it possible to do it from the component so we need to remember to do it every time we declare renderOptions. Should be straight forward when implementing a new Autocomplete when checking the console.

Changes 🔄

  • Destructure props for every renderOption instance
    • ⚠️ ImageSelect is handled here, not in this PR
  • Fix the annoying text selection highlight in Autocomplete

How to test 🧪

Verification steps

(How to verify changes)

  • Verify that all <Autocomplete /> with renderOption modified in this PR (except for ImageSelect - see description) don't display the console error when being opened

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@abailly-akamai abailly-akamai self-assigned this Oct 22, 2024
@abailly-akamai abailly-akamai marked this pull request as ready for review October 22, 2024 19:33
@abailly-akamai abailly-akamai requested a review from a team as a code owner October 22, 2024 19:33
@abailly-akamai abailly-akamai requested review from carrillo-erik, hkhalil-akamai and jaalah-akamai and removed request for a team October 22, 2024 19:33
Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

This seems like the simplest approach - thanks for doing this 🙌

Copy link

github-actions bot commented Oct 22, 2024

Coverage Report:
Base Coverage: 87.01%
Current Coverage: 87%

Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Left a concern about the theme change. Verified modified components continue to work as expected.

@abailly-akamai abailly-akamai merged commit 2145585 into linode:develop Oct 23, 2024
23 checks passed
Copy link

cypress bot commented Oct 23, 2024

Cloud Manager E2E    Run #6720

Run Properties:  status check passed Passed #6720  •  git commit 2145585478: fix: [M3-8696] - Autocomplete renderOption prop console warning (#11140)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6720
Run duration 26m 59s
Commit git commit 2145585478: fix: [M3-8696] - Autocomplete renderOption prop console warning (#11140)
Committer Alban Bailly
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 438
View all changes introduced in this branch ↗︎

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.

3 participants