Skip to content

[IconButton] add purerender mixin#1545

Merged
shaurya947 merged 1 commit intomui:masterfrom
oliviertassinari:iconbutton-pure
Sep 24, 2015
Merged

[IconButton] add purerender mixin#1545
shaurya947 merged 1 commit intomui:masterfrom
oliviertassinari:iconbutton-pure

Conversation

@oliviertassinari
Copy link
Member

Related to #1176.

@oliviertassinari
Copy link
Member Author

@shaurya947 Done.

Copy link
Member Author

Choose a reason for hiding this comment

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

ousp

shaurya947 added a commit that referenced this pull request Sep 24, 2015
@shaurya947 shaurya947 merged commit 20ddf37 into mui:master Sep 24, 2015
@shaurya947
Copy link
Contributor

Thanks @oliviertassinari!

@oliviertassinari oliviertassinari deleted the iconbutton-pure branch September 24, 2015 15:12
@oliviertassinari oliviertassinari restored the iconbutton-pure branch September 24, 2015 15:12
Copy link
Member Author

Choose a reason for hiding this comment

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

@shaurya947 I was using the context props before. I completely overlook this during the rebase! Is muiTheme working?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would say that this doesn't work. I think that we need to pass context.muiTheme instead of context to the getRelevantContextKeys in the pure-render.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, exactly what I was thinking. The argument needs to be changed in the context-pure mixin. Could you submit a PR for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

That also needs to be changed to look like this:

getRelevantContextKeys(muiTheme) {
    return {
      spacingDesktopGutterLess: muiTheme.rawTheme.spacing.desktopGutterLess,
    };
  },

and call it using this.state.muiTheme wherever it's called in the file. Please let me know if you find any more incorrect definitions of these functions in the source. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I'm gonna make a PR for this

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I've merged the new theming documentation in #1716. So make sure to rebase if you need to!

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also in src/card/card-expandable.jsx and src/text-field.jsx

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeap. Question. Is the context required to render a component?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've added you to a room in Gitter: shaurya947/context-pure-fix

Let's chat about this there.

@oliviertassinari oliviertassinari deleted the iconbutton-pure branch November 3, 2015 10:51
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Nov 10, 2020
* Rename MuiPickersUtilsPorvider => LocalizationProvider

* Improve utils typings

* Implement global format override

* [docs]  Change name of date-io customization page

* Update examples to include new wording

* Fix prop-types typescript error

* Add daetAdapter prop for passing date-io utils directly to component

* Update percy and cypress

* Unskip flaky test

* Fix typo in error message

* Fix cypress test

* One more try to fix flaky test

* Remove flaky test

* Fix visual regression scenarios tests with new version of cypress

* Run cypress tests in chrome

* Ignore dark theme change in snapshots

* Try to fix example styles one more time

* Fix inccorect name of cypress executor

* Try weird hack to reinject styles

* Optimize theme toggling for visual regression

* Update scenario names to get rid of duplications

* Rename `adapter` => `dateAdapter`

* Rename libFormats => dateFormats, libInstance => dateLibInstance

* Remove version-specific code from README.md

* Fix documentation erros in installation guide

* Run prettier on README.md

* Use edge="end" for keyboard adornment icons, closes mui#1545
@zannager zannager added the scope: icon button Changes related to the icon button. label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: icon button Changes related to the icon button.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants