-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: [DHIS2-18019] related stages UI tweaks #3872
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.
Hey @simonadomnisoru! I think most of the code looks solid here, but I have some questions about the design. Let's either discuss this internally or take it up with the design team. Other than that; very good work! 🎉
...les/capture-core/components/WidgetTwoEventWorkspace/WidgetWrapper/WidgetWrapper.container.js
Show resolved
Hide resolved
...les/capture-core/components/WidgetTwoEventWorkspace/WidgetWrapper/WidgetWrapper.container.js
Outdated
Show resolved
Hide resolved
...les/capture-core/components/WidgetTwoEventWorkspace/WidgetWrapper/WidgetWrapper.container.js
Outdated
Show resolved
Hide resolved
.../capture-core/components/Pages/EnrollmentEditEvent/PageLayout/DefaultPageLayout.constants.js
Show resolved
Hide resolved
Also, now that we're no longer bound to allowing users to define where they want this widget to be themselves, should we implement the over/under functionality the design team wanted? Open for suggestions. @simonadomnisoru @JoakimSM |
🚀 Deployed on https://deploy-preview-3872.capture.netlify.dhis2.org |
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.
Looks like a great upgrade, @simonadomnisoru! Awaiting response from Joe, but it looks good from my perspective now! 🎉
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.
LGTM, approved with minor changes.
...les/capture-core/components/WidgetTwoEventWorkspace/WidgetWrapper/WidgetWrapper.container.js
Outdated
Show resolved
Hide resolved
<div className={classes.referalResponse}> | ||
<div className={classes.linkedEvent}> | ||
<span className={classes.icon}> | ||
<IconLink16 /> |
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.
Icon color should be blue800
.
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.
Tested successfully on 2.42 version
## [101.16.4](v101.16.3...v101.16.4) (2024-11-20) ### Bug Fixes * [DHIS2-18019] related stages UI tweaks ([#3872](#3872)) ([7ea2240](7ea2240))
🎉 This PR is included in version 101.16.4 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
DHIS2-18019
Tech summary: