-
Notifications
You must be signed in to change notification settings - Fork 4
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
Enhancement/gh 61/choose templetes #73
base: development
Are you sure you want to change the base?
Conversation
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.
Overall pretty solid - looks easy to develop further when templates are supported. Just small aesthetic changes that I think would neaten up the widget. Thoughts @dromerolovo?
...ssign_commitments/commitment_template/commitment_template_widgets/commitment_list_pages.dart
Outdated
Show resolved
Hide resolved
...ents/commitment_template/commitment_template_widgets/commitment_template_expandablecard.dart
Show resolved
Hide resolved
...ents/commitment_template/commitment_template_widgets/commitment_template_expandablecard.dart
Outdated
Show resolved
Hide resolved
...ents/commitment_template/commitment_template_widgets/commitment_template_expandablecard.dart
Outdated
Show resolved
Hide resolved
...ents/commitment_template/commitment_template_widgets/commitment_template_expandablecard.dart
Outdated
Show resolved
Hide resolved
...n/assign_commitments/commitment_template/commitment_template_widgets/previousNextButton.dart
Outdated
Show resolved
Hide resolved
...s_section/assign_commitments/commitment_template/commitment_template_widgets/tag_search.dart
Outdated
Show resolved
Hide resolved
...s_section/assign_commitments/commitment_template/commitment_template_widgets/tag_search.dart
Show resolved
Hide resolved
...s_section/assign_commitments/commitment_template/commitment_template_widgets/tag_search.dart
Show resolved
Hide resolved
...ssign_commitments/commitment_template/commitment_template_widgets/commitment_list_pages.dart
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.
We already have the CommitmentItem
widget, which supports customization. It offers three different item types: simple
, addButton
, and statusChecker
, as defined by the CommitmentItemType
enumeration. Currently, the child widget is hard-coded, but maybe we can pass it as a parameter, so that it supports the CommitmentItemForm
and the data(Tags, Description, Points,). The effect that you implemented is nice, so perhaps we can incorporate it into the CommitmentItem
widget. This way, we can avoid having two similar widgets and use just one.
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.
@dromerolovo I need to ask to get it better, should I implement an expandable card widget as a third item type in the enum and add it under or a generic child type in the enum and then pass expandable card as a type of CommitmentItem where I use? Thank you so much for your attention and time.
Visit the preview URL for this PR (updated for commit fe5f757): https://collaction-development--pr73-enhancement-gh-61-ch-ieu4tmaf.web.app (expires Wed, 12 Jul 2023 12:52:00 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2cf09e8dbf51d2eab3fc6679b673d248bc8047bb |
I have created a commitment template section UI according to the Figma best of my abilities. Please review the code so I can make improvements/refactoring if necessary.