Page: Add inline rename functionality #68828
Conversation
ashharrison90
added
area/navigation
no-backport
| @@ -95,7 +96,7 @@ export function AutoSaveField<T = string>(props: Props<T>) { | |||
| disabled={disabled} | |||
| error={error || (fieldState.showError && saveErrorMessage)} | |||
| ref={fieldRef} | |||
| className={styles.widthFitContent} | |||
| className={classNames(styles.widthFitContent, restProps.className)} | |||
There was a problem hiding this comment.
😬 Not enthusiastic about supporting class name prop for new UI components.
I guess this is so we can make the input large to match the H1.... do we want/need a new lg/xl size for the text box instead? @eledobleefe
There was a problem hiding this comment.
Actually, do you even use this component here?
There was a problem hiding this comment.
oh yeah I didn't use Autosavefield in the end so can remove this change 👌
|
The time it takes for the Input to disappear after I've blurred or pressed enter feels a little bit clunky. I would prefer the frontend would directly flip to the edited title, and then flip back and display an error if it fails. |
|
@tskarhed done 👍 you're right, it feels much snappier 😍 |
There was a problem hiding this comment.
Excellent work. This is thoughly developed, tested, and thought about - well done 👏🏻
I'm a little bit uncertain on the interaction, especially with how it's just.... saved automatically without much feedback on blur, but happy to defer to others and see what it's like in practice.
(just the two small comments)
| const [isEditing, setIsEditing] = useState(false); | ||
| const [isEditInProgress, setIsEditInProgress] = useState(false); |
There was a problem hiding this comment.
I found these two names pretty confusing. Maybe we could rename isEditInProgress to isLoading?
| {/* | ||
| use localValue if it exists | ||
| this is to prevent the title from flickering back to the old value after the user has edited | ||
| caused by the delay between the save completing and the new value being refetched | ||
| */} | ||
| <H1 truncate>{localValue ?? value}</H1> |
There was a problem hiding this comment.
I think there's an iffy bit here where the parent cannot change the value prop after its ever been edited?
I think this is an instance where we want to react to changes in the value prop and sync it to state.
const [localValue, setLocalValue] = useState(value);
useEffect(() => {
setLocalValue(value);
}, [value])(this should not cause a re-render on first mount as the value doesn't change)
|
Also, long title that wraps pushes the page actions down to a new line
but then when you hit edit the actions wrap back up
|
|
@joshhunt applied your changes and adjusted the css for the title - should be more consistent now. |
ashharrison90
changed the title
ashharrison90
added
add to changelog
and removed
no-changelog
What is this feature?
EditableTitlecomponentonEditTitleprop on thePagecomponentonEditTitleis present, useEditableTitleas the page title to provide inline rename capabilitiesWhy do we need this feature?
Who is this feature for?
Which issue(s) does this PR fix?:
Fixes #68070
Special notes for your reviewer:
Please check that: