-
Notifications
You must be signed in to change notification settings - Fork 21
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
user-editable dropdowns #1661
user-editable dropdowns #1661
Conversation
for easier re-use and testing
a3e2db5
to
024fa11
Compare
…ocomplete form field component
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.spec.ts
Outdated
Show resolved
Hide resolved
...re/entity-components/entity-utils/dynamic-form-components/edit-number/edit-number.stories.ts
Outdated
Show resolved
Hide resolved
Deployed to https://pr-1661.aam-digital.net/ |
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.
Nice work!
The delay until "Add option xxx" is shown is confusing me - why did you decide against showing that in real-time?
I tuned the popup UI a bit to make it more consistent with other views. I would also like the option to cancel an edit - probably best to use reactiveForms then? (UI is there now but the logic is not because we use two-way binding currently)
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/edit-enum-popup/edit-enum-popup.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/enum-dropdown/enum-dropdown.component.html
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/edit-enum-popup/edit-enum-popup.component.ts
Outdated
Show resolved
Hide resolved
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.
Code looks pretty good now! :-)
- I could/did not test migration but already migrated instances seem stable now.
- One error on saving with newly created option:
- create a new option
- --> the new value is correctly displayed in the field
- save the entity form
- go back to list and re-open that entity's details
- --> the value is not set
- (re-selecting it now and saving seems to work)
...ttendance/dashboard-widgets/attendance-week-dashboard/attendance-week-dashboard.component.ts
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/basic-autocomplete/basic-autocomplete.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/configure-enum-popup/configure-enum-popup.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/configure-enum-popup/configure-enum-popup.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/configurable-enum/display-configurable-enum/display-configurable-enum.component.ts
Outdated
Show resolved
Hide resolved
…mplete.component.ts Co-authored-by: Sebastian <sebastian.leidig@gmail.com>
…enum-popup.component.ts Co-authored-by: Sebastian <sebastian.leidig@gmail.com>
…enum-popup.component.ts Co-authored-by: Sebastian <sebastian.leidig@gmail.com>
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.
I still see the functional issue described before:
- create a new option
- --> the new value is correctly displayed in the field
- save the entity form
- go back to list and re-open that entity's details
- --> the value is not set
- (re-selecting it now and saving seems to work)
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.
Great 👍 good to go now, I think. (I can't approve the PR here as I am the original author ...)
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
🎉 This PR is included in version 3.18.0-master.9 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 3.18.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
see issue: #1642