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

Improve renderInput spreading experience #1760

Merged
merged 3 commits into from
May 9, 2020

Conversation

dmtrKovalenko
Copy link
Member

This PR closes #1751 and closes #1758

@vercel
Copy link

vercel bot commented May 9, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mui-org/material-ui-pickers/3je15dl4t
✅ Preview: https://material-ui-pickers-git-feature-imporove-renderinput-spreading.mui-org.now.sh

@dmtrKovalenko dmtrKovalenko changed the title Improve renderInput spreading expreience Improve renderInput spreading experience May 9, 2020
@cypress
Copy link

cypress bot commented May 9, 2020



Test summary

77 0 1 0


Run details

Project material-ui-pickers
Status Passed
Commit 1534d99
Started May 9, 2020 11:39 AM
Ended May 9, 2020 11:40 AM
Duration 01:46 💡
OS Linux Debian - 9.11
Browser Chrome 78

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@codecov
Copy link

codecov bot commented May 9, 2020

Codecov Report

Merging #1760 into next will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #1760   +/-   ##
=======================================
  Coverage   90.70%   90.70%           
=======================================
  Files          77       77           
  Lines        2324     2324           
  Branches      439      439           
=======================================
  Hits         2108     2108           
  Misses        174      174           
  Partials       42       42           
Impacted Files Coverage Δ
lib/src/DateRangePicker/DateRangePickerInput.tsx 87.87% <ø> (ø)
lib/src/_shared/PureDateInput.tsx 100.00% <ø> (ø)
lib/src/_shared/hooks/useMaskedInput.tsx 96.07% <100.00%> (ø)

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 30c5bed...1534d99. Read the comment docs.

@dmtrKovalenko dmtrKovalenko merged commit 7d6525b into next May 9, 2020
@dmtrKovalenko dmtrKovalenko deleted the feature/imporove-renderInput-spreading branch May 9, 2020 11:44
alignItems: 'center',
});

function CustomInput() {
Copy link
Member

Choose a reason for hiding this comment

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

?
export default function CustomInput()

@@ -0,0 +1,28 @@
import React, { useState } from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

?
import * as React from 'react';

<TextField {...startProps} inputProps={{ 'data-mui-test': 'desktop-range-picker' }} />
<TextField
{...startProps}
inputProps={{ ...startProps.inputProps, 'data-mui-test': 'desktop-range-picker' }}
Copy link
Member

Choose a reason for hiding this comment

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

Side question, should we use a label to differentiate the two inputs instead of data-mui-test? Like label="Start date" and label="End date"?


We are providing default localized formats, but you can change this behaviour with `format` and `mask` props.
You can customize rendering input by `renderInput` prop. But make sure you will spread `ref` and `inputProps` correctly to the custom input component.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe?

The `renderInput` prop allows you to customize the rendered input.
The first argument of this render prop contains props that you need to forward.
Pay specific attention to the `ref` and `inputProps` keys.

cc @mbrookes

Copy link
Member Author

Choose a reason for hiding this comment

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

We must specify that props are https://material-ui.com/api/text-field/#textfield-api. Will refer this comment at #1761

Copy link
Member Author

Choose a reason for hiding this comment

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

By the way the type of renderInput looks to awful

(props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>

Because of core TextField typings (mui/material-ui#20716)

Copy link
Member

@oliviertassinari oliviertassinari May 10, 2020

Choose a reason for hiding this comment

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

Would a similar approach to Autocomplete work?

export interface AutocompleteRenderInputParams {
  id: string;
  disabled: boolean;
  InputLabelProps: object;
  InputProps: {
    ref: React.Ref<any>;
    className: string;
    startAdornment: React.ReactNode;
    endAdornment: React.ReactNode;
  };
  inputProps: object;
}

  /**
   * Render the input.
   *
   * @param {object} params
   * @returns {ReactNode}
   */
  renderInput: (params: AutocompleteRenderInputParams) => React.ReactNode;

@@ -0,0 +1,28 @@
import React, { useState } from 'react';
import { styled } from '@material-ui/core';
Copy link
Member

Choose a reason for hiding this comment

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

?
import { styled } from '@material-ui/core/styles';

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.

Improve renderInput props structure [docs] Document how to use a custom input with renderInput
2 participants