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

Make record name editable on show page #9172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Dec 20, 2024

When fields are not displayed in show page, title should be editable

Enregistrement.de.l.ecran.2024-12-20.a.16.04.24.mov

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR adds record name editing functionality in the show page header and breadcrumb navigation, with conditional rendering based on field visibility.

  • Added new RecordEditableName component in /pages/object-record/RecordEditableName.tsx for inline name editing with proper loading states
  • Modified RecordShowPageHeader to conditionally render editable name when layout.hideSummaryAndFields is true
  • Made Icon prop optional in NavigationDrawerInput and NavigationDrawerItem components to support breadcrumb editing
  • Added Storybook story with GraphQL mocks to test RecordEditableName rendering and interactions
  • Exposed objectMetadataItem through useRecordShowPagePagination hook to support metadata access for name editing

8 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 20 to 30
graphql.query('FindOneWorkflow', () => {
return HttpResponse.json({
data: {
workflow: {
__typename: 'Workflow',
id: '1',
name: 'My Workflow',
},
},
});
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: mock response missing mutation handler for updating the workflow name

Comment on lines +28 to +33
const { layout } = useRecordShowContainerTabs(
false,
objectNameSingular as CoreObjectNameSingular,
false,
objectMetadataItem,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: useRecordShowContainerTabs is called with hardcoded false values that could be derived from props or context

@@ -17,13 +20,33 @@ export const RecordShowPageHeader = ({
navigateToPreviousRecord,
navigateToNextRecord,
navigateToIndexView,
objectMetadataItem,
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: objectMetadataItem is now used but not type-checked - could be undefined from useRecordShowPagePagination

},
});

const [recordName, setRecordName] = useState(record?.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: recordName state initialized with record?.name before record is loaded. Could cause undefined initial state. Move this initialization into the useEffect or set an empty string as initial value.

objectMetadataItem,
);

const hasEditableName = layout.hideSummaryAndFields === true;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: layout.hideSummaryAndFields could be undefined - needs null check or default value

Comment on lines 51 to 59
const handleSubmit = (value: string) => {
updateOneRecord({
idToUpdate: objectRecordId,
updateOneRecordInput: {
name: value,
},
});
setIsRenaming(false);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

style: handleSubmit should validate that value is not empty/whitespace before updating. Also consider adding error handling for failed updates.

@thomtrp thomtrp force-pushed the tt-make-record-name-editable-in-breadcumb branch from 7c110c7 to 7c19f5e Compare December 20, 2024 17:21
@thomtrp thomtrp force-pushed the tt-make-record-name-editable-in-breadcumb branch from 7c19f5e to 9a5a87e Compare December 20, 2024 17:28
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.

1 participant