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.

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

Case tiles for case detail #33990

Merged
merged 33 commits into from
Jan 31, 2024
Merged

Case tiles for case detail #33990

merged 33 commits into from
Jan 31, 2024

Conversation

orangejenny
Copy link
Contributor

Product Description

https://dimagi-dev.atlassian.net/browse/USH-3873

The related formplayer PR has already been deployed: dimagi/formplayer#1530

Configuration in app manager:

Screen Shot 2024-01-18 at 10 05 49 AM

Web Apps UI corresponding to that configuration:

Screen Shot 2024-01-18 at 10 06 43 AM Screen Shot 2024-01-18 at 10 06 50 AM Screen Shot 2024-01-18 at 10 06 56 AM

Feature Flag

New flag: USH: Case detail tile

Safety Assurance

Safety story

New feature extending an existing flagged feature. Risk is primarily to case tiles functionality.

Automated test coverage

Tests in PR

QA Plan

https://dimagi-dev.atlassian.net/browse/QA-6030

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@orangejenny orangejenny added the product/feature-flag Change will only affect users who have a specific feature flag enabled label Jan 18, 2024
@dimagimon dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Jan 18, 2024
Copy link
Contributor

@MartinRiese MartinRiese left a comment

Choose a reason for hiding this comment

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

Still have to look through formplayer/menus/views.js. will do that later.

corehq/apps/app_manager/suite_xml/sections/details.py Outdated Show resolved Hide resolved
corehq/toggles/__init__.py Outdated Show resolved Hide resolved
@@ -13,6 +13,10 @@
// TODO: to be truly adorable, add transitions on changes to grid style, this doesn't work
transition: 2s;

&.odd-tab {
background-color: darken(@cc-bg, 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reuse the colors that Jonathan used for the groups?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These ones? #33801

I like the thought of reusing colors, but those are so dark they seem a bit overwhelming:
Screen Shot 2024-01-18 at 2 10 59 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. Agreed.

title: detailObject.get('title'),
};
if (detailObject.get('usesCaseTiles')) {
return views.CaseTileDetailView(menuData);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think CaseDetailTileView would be more descriptive if we could also rename CaseListDetailView to CaseDetailTableView

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tweaked colors ever so slightly to be more standard: 39cf74a

rowClick: function () {},
});

var initCaseTileList = function (options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. Could probably also be a const since the views are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -473,6 +473,34 @@ hqDefine("cloudcare/js/formplayer/menus/views", function () {
},
});

const CaseTileViewUnclickable = CaseTileView.extend({
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the inheritance should be the other way around since CaseTileView has more functionality but I that would probably be tricky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this makes sense, so I played around with it, but now I'm inclined to leave it as-is. I did a commit that made CaseViewUnclickable the parent view of CaseView, which is fine. CaseTileViewUnclickable can then be a child of CaseViewUnclickable, but CaseTileView needs functionality from both CaseView and CaseTileViewUnclickable which gets messy.

@orangejenny orangejenny added the awaiting QA QA in progress. Do not merge label Jan 23, 2024
@orangejenny orangejenny added QA Passed and removed awaiting QA QA in progress. Do not merge labels Jan 31, 2024
@orangejenny orangejenny merged commit a7be412 into master Jan 31, 2024
13 checks passed
@orangejenny orangejenny deleted the jls/case-tile-for-case-detail branch January 31, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/feature-flag Change will only affect users who have a specific feature flag enabled QA Passed Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants