-
-
Notifications
You must be signed in to change notification settings - Fork 219
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
[B5] Migrate first reports to bootstrap 5! #35704
Open
biyeun
wants to merge
6
commits into
master
Choose a base branch
from
bmb/b5-migrated-reports
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5c78a8c
Bootstrap 5 Migration - Completed report: CaseListReport.
biyeun 06a4c98
Bootstrap 5 Migration - Completed report: ApplicationStatusReport.
biyeun 2af6de7
"Bootstrap 5 Migration - Rebuilt diffs"
biyeun c14f536
Update corehq/apps/reports/templates/reports/filters/bootstrap5/multi…
biyeun 6fd62f7
form-control > form-select for select fields
biyeun cb274e9
"Bootstrap 5 Migration - Rebuilt diffs"
biyeun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
3 changes: 3 additions & 0 deletions
3
corehq/apps/hqwebapp/utils/bootstrap/reports/progress/bootstrap3_to_5.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Difficult to tell what changed here that is B5 related and what is code refactoring. Could those be split into separate commits or is this necessarily a complicated transformation for B5 support?
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.
the whole sorting block changed, I'm not sure I can split it up any further? There is no other way to have a diff
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.
also FYI this isn't B5 related but Datatables 1.10+ upgrade related, so there isn't going to ever be a clean-cut diff as your would expect with property changes for B5-type changes.
our old version of datatables is not compatible with bootstrap 5, which is why the upgrade was necessary. So any b5 migration of a report, will also require these kind of sorting block updates (if there is a custom sorting block) due to datatables. Relevant context is part of this PR, which we went over as a group #35648
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 guess i could have split out
_get_selected_app_sort_dict
, but that was the only refactoring related change, and I did that at the end to make the whole sorting block a little more readable. Unlikely I would have changed it from the beginning. Everything else was not a refactor