-
Notifications
You must be signed in to change notification settings - Fork 32
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
EDITOR - Organize fields into pages and sections #938
Conversation
9553beb
to
41b54c0
Compare
Affected libs:
|
📷 Screenshots are here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this first version!
The code is looking real good, didn't take the time to test.
A few remarks:
- datahub in commit messages => metadata-editor
- I like the new editor configuration, although I find it weird that each record should come with the app config. I don't know how to change it though. Maybe having a dedicated EditorConfig service appart from the fields.
And also, I was told not to put the Jira ticket tag in the PR, as this is public work.
libs/feature/editor/src/lib/components/record-form/form-field/form-field.component.ts
Outdated
Show resolved
Hide resolved
libs/feature/editor/src/lib/components/record-form/record-form.component.ts
Outdated
Show resolved
Hide resolved
libs/feature/editor/src/lib/components/record-form/record-form.component.ts
Show resolved
Hide resolved
86e584d
to
f100ab3
Compare
b0565ed
to
6357e36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a first review, I'll take more time to look into the rest.
First, thank you for the extensive work! I like the general approach and I agree that adding a pages > sections > fields hierarchical structure in the config is the right way to go.
As for the editor state, what do you think about storing the active page in the state, then instead of having a selectRecordFields
selector, provide a selectEditorSections
selector instead? Then the record-form component would get a list of sections according to the current page, and show them including the fields they contain.
That would also mean that the Breadcrumbs (or really page selector) component would be connected to the editor state directly.
apps/metadata-editor/src/app/edit/components/breadcrumbs/breadcrumbs.component.ts
Outdated
Show resolved
Hide resolved
3ab0175
to
5995a95
Compare
…2020 - Updated tsconfig.json to set target and lib to ES2020 - Ensured compatibility with modern JavaScript features like Array.prototype.flat
703b5e7
to
c1840db
Compare
c1840db
to
7ae1be7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I've made a few comments for cleaning up things, but apart from that it's good to go.
libs/feature/editor/src/lib/components/record-form/form-field/form-field.component.html
Outdated
Show resolved
Hide resolved
59284de
to
f2ae2cf
Compare
Description
This PR organize editor fields into pages and sections.
Screenshots
Quality Assurance Checklist
breaking change
labelbackport <release branch>
labelThis work is sponsored by Metadata-editor crowdfunding.