Skip to content

[TextField] Prepare removal of labelWidth prop#24595

Merged
oliviertassinari merged 1 commit into
mui:nextfrom
oliviertassinari:text-field-remove-legacy-outline
Jan 26, 2021
Merged

[TextField] Prepare removal of labelWidth prop#24595
oliviertassinari merged 1 commit into
mui:nextfrom
oliviertassinari:text-field-remove-legacy-outline

Conversation

@oliviertassinari

@oliviertassinari oliviertassinari commented Jan 24, 2021

Copy link
Copy Markdown
Member

A quick follow-up after #24563, prepare the breaking change: #22799.

@oliviertassinari oliviertassinari added the scope: text field Changes related to the text field. label Jan 24, 2021
@oliviertassinari
oliviertassinari force-pushed the text-field-remove-legacy-outline branch from 391d8b2 to 782a843 Compare January 24, 2021 14:56
@oliviertassinari
oliviertassinari force-pushed the text-field-remove-legacy-outline branch from 782a843 to 6b146ef Compare January 24, 2021 14:57
@mui-pr-bot

mui-pr-bot commented Jan 24, 2021

Copy link
Copy Markdown

No bundle size changes

Generated by 🚫 dangerJS against 6b146ef

@eps1lon eps1lon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand the purpose of this. Why not do the breaking change in one go?

@oliviertassinari

Copy link
Copy Markdown
Member Author

I didn't have the bandwidth to handle the breaking change at that point. So I went with the simplest changes.

@eps1lon

eps1lon commented Jan 26, 2021

Copy link
Copy Markdown
Member

But why roll this breaking change out partially? We never do this. Especially because it leaves code in an inconsistent state where we still use the old prop but also the new prop. Why is this change necessary?

@oliviertassinari

oliviertassinari commented Jan 26, 2021

Copy link
Copy Markdown
Member Author

@eps1lon The current changes aren't breaking, It's meant to apply changes that should have come from the time we introduced the new preferred implementation (CSS over the previous JS measuring approach).

My hope is that these changes will allow to makes the future breaking change PR more focused on removing the previous legacy implementation.

@oliviertassinari

Copy link
Copy Markdown
Member Author

I have updated #20012 to include #22799. Hopefully, it will be an easy one to handle.

@oliviertassinari
oliviertassinari merged commit abdb651 into mui:next Jan 26, 2021
@oliviertassinari
oliviertassinari deleted the text-field-remove-legacy-outline branch January 26, 2021 20:20
@eps1lon

eps1lon commented Jan 26, 2021

Copy link
Copy Markdown
Member

The current changes aren't breaking, It's meant to apply changes that should have come from the time we introduced the new preferred implementation (CSS over the previous JS measuring approach).

But they do change tests which is a clear indication of a breaking change.

Will revert the test changes in a follow-up since this was accidentally merged.

@oliviertassinari

oliviertassinari commented Jan 26, 2021

Copy link
Copy Markdown
Member Author

But they do change tests which is a clear indication of a breaking change.

The tests switch to the non legacy implementation. This implementation was untested even though we were depending on it. I believe it should have been done a long time ago.

xs9627 added a commit to xs9627/material-ui that referenced this pull request Jan 27, 2021
* next: (34 commits)
  [Tab] Migrate to emotion (mui#24651)
  [TextField] Migrate Input component to emotion (mui#24638)
  [ImageList] Migrate ImageListItemBar to emotion (mui#24632)
  [CircularProgress] Migrate to emotion (mui#24622)
  [DataTable] Add example in docs for data table (mui#24428)
  [Card] Migrate CardActionArea to emotion (mui#24636)
  [core] Fix `next` using stale pages (mui#24635)
  [List] Migrate ListItemIcon to emotion (mui#24630)
  [ClickAwayListener] Fix `children` and `onClickAway` types (mui#24565)
  [docs] Include in docs directive to silence `eslint` erroneous warning (mui#24644)
  [Fab] Migrate to emotion (mui#24618)
  [TextField] Migrate FilledInput to emotion (mui#24634)
  [Card] Migrate CardHeader to emotion (mui#24626)
  [Card] Migrate CardMedia to emotion (mui#24625)
  [ImageList] Migrate ImageListItem to emotion (mui#24619)
  [website] Add vision block (mui#24603)
  [docs] Add sorting section (mui#24637)
  [TextField] Prepare removal of labelWidth prop (mui#24595)
  [Dialog] Migrate DialogTitle to emotion (mui#24623)
  [ImageList] Migrate to emotion (mui#24615)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: text field Changes related to the text field.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants