[TextField] Add focused prop
#20276
[TextField] Add focused prop
#20276
Conversation
|
Could you explain the use case a bit more? Trapping focus needs a really strong argument since it violates WCAG guidelines. It's not even focus trapping in the classical sense (see Modals). |
Details of bundle changes.Comparing: e9fbcbf...b43f0d5 Details of page changes
|
|
@eps1lon The use case is visual only. Here is an example in Google Ads, the input has the focused styled (we are really looking for active) and yet it's the popup that has the DOM focus.
The underlying problem is: how do we inform the users if he is editing the start or end date while it's the days in the calendar that have the DOM focus? |
|
We need a visual clue for current day selection. Initial displaying is by focus, but it’s required to not loose focus after clicking next/prev month |
|
So you just want to reuse styles? That is a completely different issue that you hack around with this implementation. Could you show me the part in the date picker that needs this styling? |
|
We are moving focus to the input on day change, so aria-label will just works. |
|
There is a precedent in the SelectInput component, we shallow the blur event to maintain the focused visual state. |
import React from "react";
import { FormControl, Input, useFormControl } from "@material-ui/core";
export default function App() {
return (
<FormControl>
<Input />
<br />
<Picker />
</FormControl>
);
}
function Picker() {
const { onFocus, onBlur } = useFormControl();
return (
<select {...{ onFocus, onBlur }}>
<option>one</option>
<option>two</option>
</select>
);
}is that what you're looking for? |
|
@eps1lon No, we have After focusing (basically people with disabilities will not interact with picker, so it is not breaking a11y) |
The same standard applies for any feature request. Just because one specific use case has an easier time with prop-based composition does not mean it is the right approach or should be moved into the core. |
|
This composition is definitely useful for complex UIs and I am not intent to move pickers from TextField base approach, cause it is the most useful and cheap way for users P.S. I suppose you still don’t get the use case. |
|
So just to summarize:
|
|
OK, trying to summarize this PR intention: MotivationIn DateRangePicker we need to be able to highlight currently selected end of the range, like saying in material guidelines (direct link to the video) And it is a pretty popular approach in the other date range solutions, like react-dates. So we need to achieve highlighting of the currently selectable range end. For now, I am doing something like Without this highlight, it will be pretty confusing that selection is moved from start to end (It is possible also by direct click on the input) Proposed solutionAdd new Proposed solution with reimplementing Proposal with prop solving this problem pretty neat and quick. Possible workaroundsLeft pickers without highlighting current range end with background styling |
|
The problem with the name is that it is formulated as an action (like For a consistent API adding a The reason why I always caution against these approach is that we're mixing controlled and uncontrolled mode. Historically we know that these can cause all sorts of issues if used incorrectly. By making this a public prop we do invite incorrect usage. Without the guidelines I would actually propose a different approach: Don't highlight the textfield but rather the focused date. Otherwise the focus cursor is ambigious. But this is irrelevant. Guidelines have priority in absence of other specs.
I'm not following that part. This seems to imply customizing the TextField is a bad idea in the first place? |
|
About UX of this preview: But it still confusing case when you are hovering the date before start, while selecting the end Related to TextField: So what?So what is your proposal? Make |
|
@eps1lon could you please request any changes you think needs to be done, so I will be able to finish this PR? |
|
Changed name to |
|
I have tried to benchmark all the Google's date range pickers I could get my hands one. Google Analytics
input focused
calendar focused Google Flight
input focused
calendar focused Google Ads
input focused
calendar focused I think that we can move forward with the proposal of @eps1lon
Regarding the mixing of an uncontrolled and controlled state, I think that we can address this later down the road. Right now, there is no obvious problem this will cause. However, if it does, we know how to solve it, there are no blockers I'm aware of that might stuck us in the future.
@dmtrKovalenko Going with a different name to represent a different state sounds like a viable alternative. Maybe |
oliviertassinari
added
component: text field
oliviertassinari
changed the title
forceFocus prop focus prop
dmtrKovalenko
changed the title
focus prop focused prop
|
@dmtrKovalenko While mixing a controlled and uncontrolled behavior with |
As discussed with @oliviertassinari this needs for date range picker to display currently selectable range end, according to material design guideline. And this prop is also can be useful for public users.