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.

Sign up for GitHub

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

Jump to bottom

fix: [DHIS2-17054] handle empty expression value #3635

Merged
merged 4 commits into from
Jun 6, 2024
Merged

fix: [DHIS2-17054] handle empty expression value #3635

merged 4 commits into from
Jun 6, 2024

Conversation

simonadomnisoru
Copy link
Contributor

@simonadomnisoru simonadomnisoru commented May 8, 2024 •
edited by jira bot

DHIS2-17054

Tech summary

  • handle empty expression value in processDisplayKeyValuePair
  • added a unit test for the undefined value use case
  • small CSS adjustments

@simonadomnisoru simonadomnisoru requested a review from a team as a code owner May 8, 2024 09:54
superskip
superskip reviewed May 10, 2024
View reviewed changes
Copy link
Contributor

@superskip superskip left a comment

Choose a reason for hiding this comment

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

I found a small simplification, posted as comments. Regarding the css changes, will the bottom margin be constant 0 now? How did it work previously (what does '& + &' mean)?

packages/rules-engine/src/processors/rulesEffectsProcessor/rulesEffectsProcessor.js Outdated Show resolved Hide resolved
packages/rules-engine/src/processors/rulesEffectsProcessor/rulesEffectsProcessor.js Outdated Show resolved Hide resolved
simonadomnisoru and others added 2 commits May 27, 2024 14:25
@simonadomnisoru @superskip
chore: simplify logic
dfca7d1 
Co-authored-by: Tony Valle <79843014+superskip@users.noreply.github.com>
@simonadomnisoru @superskip
chore: simplify logic
55c7840 
Co-authored-by: Tony Valle <79843014+superskip@users.noreply.github.com>
@simonadomnisoru
Copy link
Contributor Author

I found a small simplification, posted as comments. Regarding the css changes, will the bottom margin be constant 0 now? How did it work previously (what does '& + &' mean)?

Hey,
I applied the small simplifications you suggested. As for the bottom margins, yes, they are all constant now as requested in the ticket.
From what I understand from next sibling combinator and nesting selector, & + & compiles to .listItem + .listItem and matches from the second .listItem class until the last one. This means that before the first listItem had a different bottom margin than the second and the rest ones.
Thanks for the feedback!

@superskip
Copy link
Contributor

From what I understand from next sibling combinator and nesting selector, & + & compiles to .listItem + .listItem and matches from the second .listItem class until the last one. This means that before the first listItem had a different bottom margin than the second and the rest ones.

Thanks for the explanation :)

@CooperJoe Do you recall what you tried to achieve by using the & + & selector?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 5, 2024

🚀 Deployed on https://deploy-preview-3635--dhis2-capture.netlify.app

geethaalwan
geethaalwan requested changes Jun 6, 2024
View reviewed changes
Copy link

@geethaalwan geethaalwan left a 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,2.41.1,2.40.4,2.39.6 versions

@simonadomnisoru simonadomnisoru merged commit 08291fa into master Jun 6, 2024
41 of 44 checks passed
@simonadomnisoru simonadomnisoru deleted the DHIS2-17054 branch June 6, 2024 11:09
dhis2-bot added a commit that referenced this pull request Jun 6, 2024
@dhis2-bot
chore(release): cut 100.68.22 [skip release]
b1fed10 
## [100.68.22](v100.68.21...v100.68.22) (2024-06-06)

### Bug Fixes

* [DHIS2-17054] handle empty expression value ([#3635](#3635)) ([08291fa](08291fa))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.68.22 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geethaalwan geethaalwan geethaalwan approved these changes

@superskip superskip superskip approved these changes

Assignees
No one assigned
Labels
released testing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@simonadomnisoru @superskip @dhis2-bot @geethaalwan