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

Convert substr() to substring() #6954

Merged
merged 4 commits into from
Jul 25, 2023
Merged

Convert substr() to substring() #6954

merged 4 commits into from
Jul 25, 2023

Conversation

wonseop
Copy link
Contributor

@wonseop wonseop commented Jul 17, 2023

Summary

String.prototype.substr() is deprecated.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substr Therefore, I changed the code to use String.prototype.substring() instead of String.prototype.substr().

QA

General checklist

  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox

String.prototype.substr() is deprecated.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substr
Therefore, I changed the code to use String.prototype.substring() instead of String.prototype.substr().
@kibanamachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@tkajtoch
Copy link
Member

@wonseop Thank you for your submission! I just recently noticed these and wanted to fix them bt you were faster 🏃🏻

I plan to review and test your code today or tomorrow.

@tkajtoch tkajtoch self-requested a review July 18, 2023 20:15
@tkajtoch
Copy link
Member

I tested the changes and they work great!

@wonseop Could you please add a changelog entry for your changes? You can do so by running yarn yo-changelog 6954 and commiting the result.

@wonseop
Copy link
Contributor Author

wonseop commented Jul 20, 2023

@tkajtoch Thank you for reviewing! I added a changelog.

Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

The changes look great, thank you!

@tkajtoch
Copy link
Member

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6954/

@cee-chen cee-chen enabled auto-merge (squash) July 24, 2023 19:27
@cee-chen cee-chen disabled auto-merge July 25, 2023 14:36
@cee-chen cee-chen merged commit 581cf6d into elastic:main Jul 25, 2023
mistic pushed a commit to elastic/kibana that referenced this pull request Aug 3, 2023
`85.0.0` ➡️ `85.1.0`

⚠️ The biggest change in this PR by far is the **removal** of several
`EuiFilterSelectItem` CSS classes and styles associated with those
classes. EUI has previously exported component-specific CSS classes for
usage such as `.euiFilterSelect__items`,
`.euiFilterGroup__popoverPanel`, or `.euiAccordionForm`.

❌ **As of our move to CSS-in-JS, we are actively moving away from this
architecture**. The goal is to either provide either a component or prop
directly to you instead of a CSS class. As a reminder, our classNames
are not guaranteed APIs and are subject to change without warning -
should you need to tweak or customize EUI's styling, we strongly
recommend passing in your own `className`.

💬 Moving forward, EUI will attempted to convert any usages of removed
CSS classes and their direct usages in Kibana for you. In most cases,
we'll hopefully be able to take the correct path of using a component or
prop instead. In cases where we cannot or significant/complex changes
are required, we may temporarily convert the CSS to a static CSS-in-JS
usage instead and add a TODO asking the relevant team to address this in
their own time (for example, the deprecation of `EuiFilterSelectItem`
and its conversion to `EuiSelectable`).

## [`85.1.0`](https://github.com/elastic/eui/tree/v85.1.0)

- Updated `EuiComboBox`'s `options` to accept `option.append` and
`option.prepend` props
([#6953](elastic/eui#6953))
- Updated deprecated `.substr()` usages to `.substring()`
([#6954](elastic/eui#6954))
- Updated `EuiInlineEdit`'s read mode button to include a title tooltip,
increasing readability of truncated text
([#6966](elastic/eui#6966))

**Bug fixes**

- Fixed `EuiFilterGroup`'s responsive styles
([#6983](elastic/eui#6983))

**Deprecations**

- Deprecated `EuiFilterSelectItem`; Use `EuiSelectable` instead
([#6982](elastic/eui#6982))

**CSS-in-JS conversions**

- Converted `EuiFilterSelectItem` to Emotion
([#6982](elastic/eui#6982))
- Removed `.euiFilterSelect__items` CSS; Use `EuiSelectable` instead
([#6982](elastic/eui#6982))
- Removed `.euiFilterSelect__note` and `.euiFilterSelect__noteContent`
CSS; Use `EuiSelectableMessage` instead
([#6982](elastic/eui#6982))
- Added `focus.transparency` and `focus.backgroundColor` theme tokens
([#6984](elastic/eui#6984))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants