feat(TextInput): add helper util for left trim by jenny-s51 · Pull Request #4691 · patternfly/patternfly-react

jenny-s51 · 2020-08-13T19:42:49Z

What: Closes #4637

This is WIP; working on adding this functionality to TextInput.

patternfly-build · 2020-08-13T19:52:26Z

PF4 preview: https://patternfly-react-pr-4691.surge.sh
A11y report: https://patternfly-react-pr-4691-coverage.surge.sh

codecov-commenter · 2020-08-21T14:32:18Z

Codecov Report

Merging #4691 into master will decrease coverage by 0.22%.
The diff coverage is 20.33%.

@@            Coverage Diff             @@
##           master    #4691      +/-   ##
==========================================
- Coverage   52.52%   52.30%   -0.23%     
==========================================
  Files         514      514              
  Lines        8974     9030      +56     
  Branches     3266     3281      +15     
==========================================
+ Hits         4714     4723       +9     
- Misses       3673     3718      +45     
- Partials      587      589       +2

Flag	Coverage Δ
#patternfly4	`52.30% <20.33%> (-0.23%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/react-core/src/helpers/util.ts	`37.50% <10.00%> (-9.17%)`	⬇️
.../react-core/src/components/TextInput/TextInput.tsx	`46.34% <31.03%> (-40.33%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffe4695...5741c9d. Read the comment docs.

jschuler · 2020-08-21T15:24:58Z

@@ -26,7 +26,7 @@ class SimpleTextInput extends React.Component {
    const { value } = this.state;


Undo changes to this file

kmcfaul

lgtm

jschuler · 2020-08-21T15:27:07Z

Also, can you add a separate example to TextInput.md instead of modifying the Disabled example?

jschuler · 2020-08-21T16:38:07Z

+  componentDidMount() {
+    if (this.props.isLeftTruncated) {
+      this.handleResize();
+      window.addEventListener('resize', debounce(this.handleResize, 250));


i just realized that the input box dimensions can change even if there was no window resize, for example if a sidebar is opened. Perhaps we should look at something like the ResizeObserver instead
https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver
Example implementation
https://github.com/ZeeCoder/use-resize-observer

Opened followup issue for next milestone here #4710 @jenny-s51 @tlabaj

jschuler · 2020-08-21T17:19:43Z

      isRequired,
      isDisabled,
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      onFocus,


you'll want to extract isLeftTruncated here even though it is not used in the returned JSX so it doesn't end up in the DOM. Can combine the 3 unused props in this way

/* eslint-disable @typescript-eslint/no-unused-vars */ isLeftTruncated, onFocus, onBlur, /* eslint-enable @typescript-eslint/no-unused-vars */

mcarrano

So how would this work for different components? I can see it implemented for the Text input, but what if I wanted on a different component like a Table or a Select? Would it need to be applied on a per component basis or is there a general way to do this? Also, we typically present a tooltip for truncated text strings, not sure if that would be part of the component demo or not.

jschuler · 2020-08-21T18:03:54Z

@mcarrano with this PR there will be some utility functions to make it easier to implement for others, but we'll have to add and test it per component.
Adding tooltip would be possible but the examples already didn't have it to begin with (e.g. if you type more text into the first input example until there is overflow with the ellipsis on the right there is no tooltip at the moment).
Also it is not quite clear to me if there should be tooltips on enabled inputs since you can see and edit the whole string once you focus on it. I can see it being more useful for disabled inputs, but that might take more work as well since disabled elements generally don't emit events.

mcarrano · 2020-08-21T18:26:07Z

Good points @jschuler . Are the utility functions documented somewhere? If I were a developer wanting to apply this on my own to a different component. Would it be obvious how to do that or would they need to wait for us to add it as a new library update?

jschuler · 2020-08-21T18:42:04Z

@mcarrano Currently they are only documented in code! But we do export all the functions out so consumers could use them if they wanted to. I think the utility function would be pretty simple to use!

import { trimLeft } from '@patternfly/react-core'
...
const myElement = document.getElementById("my-input");
trimLeft(myElement, "The string which should be truncated on the left if needed");

tlabaj

Can you just update demo app and integration test please.

jschuler

LGTM

jschuler · 2020-08-24T14:17:09Z

LGTM, I noticed a bug where if you remove isDisabled / isRead and try to type into the textbox it won't let you! But it is not something from this PR, it already exists since the last release at least. But we should fix this @tlabaj

Nevermind, turns out the value has to be changed via state

jschuler · 2020-08-24T14:21:18Z

+export const trimLeft = (node: HTMLElement, value: string) => {
+  const availableWidth = innerDimensions(node).width;
+  let newValue = value;
+  if (getTextWidth(value, node) > availableWidth) {


just one bug here, can you add an else condition to this if. Noticed that if you just a little overflow and you expand the textbox until the whole text should fit again, it won't restore the value

else { if ((node as HTMLInputElement).value) { (node as HTMLInputElement).value = value; } else { node.innerText = value; } }

tlabaj

LGTM

patternfly-build · 2020-08-24T15:44:05Z

Your changes have been released in:

@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]

Thanks for your contribution! 🎉

jenny-s51 marked this pull request as draft August 13, 2020 19:43

jenny-s51 force-pushed the iss4637 branch from 4e7fb59 to 2dba8b5 Compare August 17, 2020 20:08

jenny-s51 commented Aug 18, 2020

View reviewed changes

Comment thread packages/react-core/src/helpers/util.ts Outdated

Comment thread packages/react-core/src/helpers/util.ts Outdated

Comment thread packages/react-core/src/components/TextInput/examples/TextInput.md Outdated

jenny-s51 requested a review from jschuler August 18, 2020 18:54

jenny-s51 marked this pull request as ready for review August 21, 2020 14:43

jschuler suggested changes Aug 21, 2020

View reviewed changes

kmcfaul previously approved these changes Aug 21, 2020

View reviewed changes

jenny-s51 dismissed kmcfaul’s stale review via adea688 August 21, 2020 16:05

tlabaj added the ux review label Aug 21, 2020

tlabaj requested a review from mcarrano August 21, 2020 16:32

jschuler reviewed Aug 21, 2020

View reviewed changes

mcarrano reviewed Aug 21, 2020

View reviewed changes

jenny-s51 changed the title ~~feat(text): add helper util for left trim~~ feat(TextInput): add helper util for left trim Aug 21, 2020

tlabaj suggested changes Aug 21, 2020

View reviewed changes

jenny-s51 and others added 9 commits August 21, 2020 16:26

feat(text): adds helper util for left trim

98d1b4e

fix headings example

24d1bfa

get by classname instead of id

0addd16

add case for when there are no childNodes, WIP

876fae6

use ceil func and make input val a state var

6113521

input trim left

5ef58fc

fix innerRef uses

f385eac

fix for FF

7409c06

cleanup

7bebf07

jschuler and others added 7 commits August 21, 2020 16:26

types

930aae2

update snapshots and fix linting

26dc6c5

update snapshots

43af08a

address PR feedback

5888cdd

extract isLeftTruncated

7728df0

add demo and integration test

be6cc58

update prop name

f18ad3c

jenny-s51 force-pushed the iss4637 branch from 72aaa55 to f18ad3c Compare August 21, 2020 20:26

update prop name

b3a5316

jschuler reviewed Aug 21, 2020

View reviewed changes

jenny-s51 and others added 2 commits August 21, 2020 16:55

Delete PageBreadcrumbAndTertiaryNavDemo.tsx

a120a5d

use prop directly on textInput component

62ed831

jschuler previously approved these changes Aug 22, 2020

View reviewed changes

jenny-s51 dismissed jschuler’s stale review via e6c5b45 August 22, 2020 01:18

jenny-s51 requested review from jschuler, kmcfaul, mcarrano and tlabaj August 24, 2020 13:45

jschuler suggested changes Aug 24, 2020

View reviewed changes

PR feedback

8776afa

jenny-s51 force-pushed the iss4637 branch from e6c5b45 to 8776afa Compare August 24, 2020 14:44

jenny-s51 added 2 commits August 24, 2020 10:48

fix demo

d406049

add back ref

f02bfbb

jenny-s51 requested a review from jschuler August 24, 2020 15:14

tlabaj approved these changes Aug 24, 2020

View reviewed changes

jschuler approved these changes Aug 24, 2020

View reviewed changes

jschuler merged commit 44dba19 into patternfly:master Aug 24, 2020

jenny-s51 deleted the iss4637 branch August 24, 2020 15:47

		@@ -26,7 +26,7 @@ class SimpleTextInput extends React.Component {
		const { value } = this.state;

Conversation

jenny-s51 commented Aug 13, 2020

Uh oh!

patternfly-build commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 21, 2020

Codecov Report

Uh oh!

Uh oh!

jschuler Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

kmcfaul left a comment

Choose a reason for hiding this comment

Uh oh!

jschuler commented Aug 21, 2020

Uh oh!

jschuler Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

jschuler Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

jschuler Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

jschuler commented Aug 21, 2020

Uh oh!

mcarrano commented Aug 21, 2020

Uh oh!

jschuler commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jschuler left a comment

Choose a reason for hiding this comment

Uh oh!

jschuler commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jschuler Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Aug 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

patternfly-build commented Aug 13, 2020 •

edited

Loading

jschuler commented Aug 21, 2020 •

edited

Loading

jschuler commented Aug 24, 2020 •

edited

Loading