Skip to content

feat(ui-editable): rework InPlaceEdit and Editable#2483

Merged
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4969-in-place-edit-rework-multi
Apr 1, 2026
Merged

feat(ui-editable): rework InPlaceEdit and Editable#2483
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4969-in-place-edit-rework-multi

Conversation

@ToMESSKa
Copy link
Copy Markdown
Contributor

@ToMESSKa ToMESSKa commented Mar 20, 2026

INSTUI-4969

ISSUE:

  • InPlaceEdit needs to be migrated to the new theming system
  • (Editable does not need to be migrated as it has no versioned import, tokens or icons)

TEST PLAN:

  • check if the component works the same as before in v1
  • check if the components and the examples work in v2 (especially the dark theme)
  • check if all necessary the imports are renamed to latest in v2
  • check if the test files are removed from the v1
  • check if the right imports are used in exports/b.ts and exports/a.ts
  • check if the package.json uses the correct versioning
  • check if the renamed or removed tokes are documented in the upgrade guide (if there are any)
  • check if the index.ts files use withStyle instead of withStyleLegacy
  • check if the v2 component uses all the tokens
  • check if all the old icons were replace with the new ones in the index.tsx, test and README files

@ToMESSKa ToMESSKa self-assigned this Mar 20, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-04-01 14:53 UTC

@ToMESSKa ToMESSKa requested review from git-nandor and matyasf March 20, 2026 16:03
Copy link
Copy Markdown
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

in both InPlaceEdit and Editable the examples have really low contrast with the dark theme:

Image Image

As I see just the example needs updating, otherwise it looks good!

@ToMESSKa ToMESSKa force-pushed the INSTUI-4969-in-place-edit-rework-multi branch from e43a047 to 38f54a9 Compare March 26, 2026 16:08
borderRadius: 0,
padding: 0,
margin: 0,
color: 'inherit',
Copy link
Copy Markdown
Contributor Author

@ToMESSKa ToMESSKa Mar 26, 2026

Choose a reason for hiding this comment

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

Removing this fixed the hard to see text in the dark theme in InPlaceEdit
If you try this code, and click on the editable text , the text will turn red, while the same code will not work in v11_6 because the text color was inherited instead of using the Text component's warning color through the renderEditor prop.

const Example = (props) => {
    const [mode, setMode] = useState(props.mode || 'view')
    const [value, setValue] = useState('This is some text')
    const [inline, setInline] = useState(true)

    // You must provide this to Editable to be
    // notified of mode changes
    const handleChangeMode = (mode) => {
      setMode(mode)
    }

    // You attach an event handler to your edit component
    // to be notified of value changes from user interactions
    const handleChange = (event) => {
      setValue(event.target.value)
    }

    // Renders the view component
    // Be sure to give it the current value
    const renderView = () => (
      <Text
        color={value ? 'primary' : 'secondary'}
        weight={value ? 'normal' : 'light'}
        size="large"
      >
        {value || 'Enter some text'}
      </Text>
    )

    // Renders the edit component.
    // You have to forward the props on, which
    // includes an onBlur property to help manage
    // the mode changes.
    // Be sure to give it the current value
    const renderEdit = ({ onBlur, editorRef }) => (
      <Text
        color="warning"
        size="large"
        as="input"
        type="text"
        value={value}
        onChange={handleChange}
        aria-label="The title"
        onBlur={onBlur}
        elementRef={editorRef}
      />
    )

    // Renders the edit button.
    // Leverage the default implementation provided by InPlaceEdit
    const renderEditButton = (props) => {
      props.label = `Edit title "${value}"`
      return InPlaceEdit.renderDefaultEditButton(props)
    }

    const onChangeLayout = (event) => {
      setInline(event.target.checked)
    }

    return (
      <View as="div">
        <InPlaceEdit
          renderViewer={renderView}
          renderEditor={renderEdit}
          renderEditButton={renderEditButton}
          onChangeMode={handleChangeMode}
          mode={mode}
          value={value}
          inline={inline}
        />
        <View as="div" margin="small 0">
          <Checkbox label="inline" checked={inline} onChange={onChangeLayout} />
        </View>
      </View>
    )
  }

  render(<Example />)

@ToMESSKa
Copy link
Copy Markdown
Contributor Author

ToMESSKa commented Mar 26, 2026

in both InPlaceEdit and Editable the examples have really low contrast with the dark theme:

Image Image
As I see just the example needs updating, otherwise it looks good!

@matyasf I fixed both. For the InlineEdit fix, see my comment.

@ToMESSKa ToMESSKa requested a review from matyasf March 27, 2026 08:12
@ToMESSKa ToMESSKa requested a review from hajnaldo March 31, 2026 08:17
@ToMESSKa ToMESSKa changed the title feat(ui-editable): rework InPlaceEdit feat(ui-editable): rework InPlaceEdit and Editable Mar 31, 2026
@ToMESSKa ToMESSKa force-pushed the INSTUI-4969-in-place-edit-rework-multi branch from 38f54a9 to ecb15de Compare April 1, 2026 07:56
BREAKING CHANGE: contains breaking changes due to component using the new theming system

INSTUI-4969
Copy link
Copy Markdown
Collaborator

@hajnaldo hajnaldo left a comment

Choose a reason for hiding this comment

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

Editable:

  1. Is the border around the field a focus ring when it is active?
    If so, it should have the same color as the general focus ring
  2. In dark mode, the bg and the text itself is the same as in light mode when the field is active. It should have a dark background and light text (similarly to the text input)

@ToMESSKa ToMESSKa force-pushed the INSTUI-4969-in-place-edit-rework-multi branch from ecb15de to 9b5a5bd Compare April 1, 2026 11:53
@ToMESSKa
Copy link
Copy Markdown
Contributor Author

ToMESSKa commented Apr 1, 2026

Editable:

  1. Is the border around the field a focus ring when it is active?
    If so, it should have the same color as the general focus ring
  2. In dark mode, the bg and the text itself is the same as in light mode when the field is active. It should have a dark background and light text (similarly to the text input)

@hajnaldo I fixed both of these, can you please check them?

@ToMESSKa ToMESSKa requested a review from hajnaldo April 1, 2026 12:08
@ToMESSKa ToMESSKa merged commit 10f637b into master Apr 1, 2026
11 of 12 checks passed
@ToMESSKa ToMESSKa deleted the INSTUI-4969-in-place-edit-rework-multi branch April 1, 2026 14:53
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.

4 participants