7093 add sort toggle to prevent report filter dropdowns overlapping #7406

aimee-mcneil-melville · 2025-04-17T03:32:25Z

👩🏻‍💻 What does this PR do?

Replaced the Sort Direction dropdown selection in report arguments with a new SortToggle component (Json forms component), which displays buttons for Ascending and Descending sorting

With some testing the original issue was found to be due to android defaults in relation to the keyboard:
When the keyboard is closed whilst a dropdown is open, the dropdown direction goes down. Then selecting another dropdown, the keyboard goes up. The dorwdowns then seen to get confused with the direction changes and overlap.

Rather than changing default behaviour of keyboards or dropdowns, I thought it would be a nicer user experience to have buttons for the sorting direction - it will only be one of two options, and is easier for the user to access. This also solves the overlapping - if there aren't two dropdowns next to each other, they can't overlap.

The selection behaviour is done through state, and selecting an already selected button will unselect it. I looked into the MUI ToggleButton as they use an exclusive prop - however didn't see any benefit to creating this component when they also used state to manage this.

First go at the styling - can have another pass with suggestions. I used the flat button so it didn't visually compete with the dialog buttons, and as it is not performing an action visible immediately to the user that made more sense to me.

💌 Any notes for the reviewer?

This issue could still arise in the future if more dropdown fields are added to report arguments
This icon is the existing one for sorting (used in table headers). MUI doesn't have an icon in a similar design to the one suggested by Roxy - this could be looked at in future to either create custom SVGs or import icons from elsewhere
This icon also does not take a color - black and white only (Unless I missed something there).
The more I looked the more inconsistencies I found in the form alignments 😢

In all forms the labels are aligned to 'top' instead of horizontally centered like elsewhere in the app.
There is a mix of left aligned fields and right aligned fields. Most fields have padding-right to adjust widths, except for sort-by and sort direction. This was most apparent in the expiring items report.
I didn't find any quick fixes for the expiring items alignment, as I got further into it I figured it should be a separate issue.

Are there any custom report that are affected by this bug, or could benefit from using this component?

🧪 Testing

from /server: cargo build then run ./target/debug/remote_server_cli build-reports && ./target/debug/remote_server_cli upsert-reports -o
Go to Reports -> for each of the 'Stock & Items' and 'Expiring' reports, open the arguments modal and enter some sorting criteria
Check that a selection can be unselected
Check the ascending and descending buttons cannot both be selected at once
Click Ok -> check that any selections made are correct in the resulting report

📃 Documentation

Part of an epic: documentation will be completed for the feature as a whole
No documentation required: no user facing changes or a bug fix which isn't a change in behaviour
These areas should be updated or checked:
1. There is no mention of sorting in the docs - should this be added?

📃 Reviewer Checklist

The PR Reviewer(s) should fill out this section before approving the PR

Breaking Changes

No Breaking Changes in the Graphql API
Technically some Breaking Changes but not expected to impact any integrations

Issue Review

All requirements in original issue have been covered
A follow up issue(s) have been created to cover additional requirements

Tests Pass

Postgres
SQLite
Frontend

github-actions · 2025-04-17T03:37:18Z

Bundle size difference

Comparing this PR to main

Old size	New size	Diff
6.08 MB	6.08 MB	1.09 KB (0.02%)

aimee-mcneil-melville · 2025-04-17T04:14:43Z

client/packages/programs/src/JsonForms/common/components/SortToggle.tsx

+    sortDirection === value ? selectedStyles : undefined;
+
+  const handleClick = (value: SortDirection) => {
+    const newValue = sortDirection === value ? null : value;


This line with newValue ensures that only one button can be selected at a time - deselecting the button if it’s already selected, or selecting it if it’s not. This is what the MUI ToggleButton propexclusive achieves, whilst using common components we already have

aimee-mcneil-melville · 2025-04-17T04:16:10Z

client/packages/programs/src/JsonForms/common/components/SortToggle.tsx

+        <>
+          <FlatButton
+            label={t('report.ascending')}
+            onClick={() => handleClick('asc')}


Passing in 'asc' as the value, as FlatButton does not have a value attribute. Works in this scenario where only two options exist

aimee-mcneil-melville · 2025-04-17T04:17:21Z

client/packages/programs/src/JsonForms/common/components/SortToggle.tsx

+    return null;
+  }
+
+  const selectedStyles = (theme: Theme) => ({


custom styling for when the button is the selected option

aimee-mcneil-melville · 2025-04-17T04:18:17Z

standard_reports/expiring-items/latest/argument_schemas/arguments.json

@@ -26,8 +26,8 @@
        },
        "dir": {
          "description": "sort by dir",
-          "type": "string",
-          "enum": ["asc", "desc"]
+          "type": ["string", "null"],


Included 'null' here to prevent console errors when deselecting

Chris-Petty

The good news is that the new buttons look good. The bad news is that sorting doesn't work?

Chris-Petty · 2025-04-27T23:38:22Z

standard_reports/item-usage/latest/argument_schemas/arguments.json

@@ -35,8 +35,8 @@
        },
        "dir": {


tangentially I find it funny that we shorten this to dir, meanwhile having properties like "monthlyConsumptionLookBackPeriod". Also just curious is the "description" just for documenting in this JSON or is it shown somewhere?

Yes that is quite a difference between the property names! It's not immediately obvious (at least to me) what 'dir' represents. I'd be happy to update that to 'direction'
The description is used for internal/documentation only atm, though it looks like another use case is for a Tooltip

Honestly the reason I even commented is it took me a second too - I was like wait directory?

Nah maybe a refactor issue if anything heh.

Yea I thought description might be a tool tip, but then it'd need localisation! Maybe it should be regardless, but the use case isn't really obvious heh.

Thanks Chris, I have raised in #7546

Chris-Petty · 2025-04-28T00:16:16Z

client/packages/programs/src/JsonForms/common/components/SortToggle.tsx

Looks good. If I could request thinking about one thing, it's that this is pretty hard coded to the sorting use case. A more generic and flexible component could probably be made of this (perhaps in the future)

Thanks for bringing that up, I should have covered that in the PR!
I did consider this when creating it, and went with the most straight forward solution to fix the issue. With sorting direction only having two possible options, and without another use case in mind, it made sense to me to hardcode for this case

There is an existing forms pattern with 'options' like in the 'sort-by' dropdown below - to make that pattern work for a flexible toggle button setup it would need at least:

logic in the toggle component to get the icon and render to correct icon/label pairing

logic to get and set the 'value' as it is not built in to the FlatButton

considerations for control over the number of options that could theoretically be added and/or new logic plus styling if able to add more than two (more like a multi-select)

open-msupply/standard_reports/item-usage/latest/argument_schemas/arguments_ui.json

Lines 31 to 46 in 35917ab

"options": {

"show": [

["name", "T#report.item-name"],

["code", "T#label.code"],

["SOH", "T#report.stock-on-hand"],

["MOS", "T#report.months-cover"],

["monthConsumption", "T#report.consumption-month"],

["lastMonthConsumption", "T#report.consumption-last-month"],

["twoMonthsAgoConsumption", "T#report.consumption-two-months-ago"],

["expiringInSixMonths", "T#report.expiring-6-months"],

["expiringInTwelveMonths", "T#report.expiring-12-months"],

["stockOnOrder", "T#report.stock-on-order"],

["AMC12", "T#report.amc-12-months"],

["AMC24", "T#report.amc-24-months"]

]

}

Yea with 1-3 short options (especially if icons and no text) toggle is good, <=~5 radio buttons (especially if text) and beyond that is combobox.

aimee-mcneil-melville · 2025-04-28T05:03:20Z

Sorting should be consistent now - it was a gotcha with state. Thanks for picking up on that!
LMK if I should rename all 'dir's to 'direction' on all of the forms as well

…msupply into 7093-report-filter-dropdowns-overlapping

Chris-Petty

fixed woo thanks

Chris-Petty · 2025-04-29T22:49:07Z

client/packages/programs/src/JsonForms/common/components/SortToggle.tsx

@@ -38,7 +38,7 @@ const UIComponent = (props: ControlProps) => {
  const handleClick = (value: SortDirection) => {
    const newValue = sortDirection === value ? null : value;
    setSortDirection(newValue);
-    handleChange(path, sortDirection);
+    handleChange(path, newValue);


oh hahahaha ya that'd do it

aimee-mcneil-melville added 6 commits April 16, 2025 15:52

add sortToggle form component

89f94a3

use sortToggle in expiring items

da31b8c

change to flatbutton with styling on selection

acfbdd3

translations and more styling

2cdc675

add sort to standard reports schemas

ef440ba

fix import

9453b77

github-actions bot added this to the V2.7 Cooldown ( v2.8 release) milestone Apr 17, 2025

github-actions bot added platform: android Team Ruru 🦉 Roxy, Noel, Chris, Aimee, Laché Severity: Normal Bugs which have an acceptable workaround. Moderate/tolerable user impact. Next minor release. Needs Investigation Not sure what's the cause of issue feature: reports labels Apr 17, 2025

aimee-mcneil-melville commented Apr 17, 2025

View reviewed changes

Chris-Petty self-assigned this Apr 27, 2025

Chris-Petty requested changes Apr 28, 2025

View reviewed changes

fix toggled state

dc97264

Merge branch 'develop' of https://github.com/msupply-foundation/open-…

a83ec1a

…msupply into 7093-report-filter-dropdowns-overlapping

aimee-mcneil-melville requested a review from Chris-Petty April 28, 2025 23:42

Chris-Petty approved these changes Apr 29, 2025

View reviewed changes

aimee-mcneil-melville merged commit 36596ed into develop Apr 30, 2025
5 checks passed

aimee-mcneil-melville deleted the 7093-report-filter-dropdowns-overlapping branch April 30, 2025 20:57

aimee-mcneil-melville mentioned this pull request Apr 30, 2025

JSON Forms Properties Renaming (dir -> directions) #7546

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7093 add sort toggle to prevent report filter dropdowns overlapping #7406

7093 add sort toggle to prevent report filter dropdowns overlapping #7406

aimee-mcneil-melville commented Apr 17, 2025 •

edited

Loading

github-actions bot commented Apr 17, 2025 •

edited

Loading

aimee-mcneil-melville Apr 17, 2025

aimee-mcneil-melville Apr 17, 2025

aimee-mcneil-melville Apr 17, 2025

aimee-mcneil-melville Apr 17, 2025

Chris-Petty left a comment

Chris-Petty Apr 27, 2025

aimee-mcneil-melville Apr 28, 2025

Chris-Petty Apr 29, 2025

aimee-mcneil-melville Apr 30, 2025

Chris-Petty Apr 28, 2025

aimee-mcneil-melville Apr 28, 2025

Chris-Petty Apr 29, 2025

aimee-mcneil-melville commented Apr 28, 2025

Chris-Petty left a comment

Chris-Petty Apr 29, 2025

	"options": {
	"show": [
	["name", "T#report.item-name"],
	["code", "T#label.code"],
	["SOH", "T#report.stock-on-hand"],
	["MOS", "T#report.months-cover"],
	["monthConsumption", "T#report.consumption-month"],
	["lastMonthConsumption", "T#report.consumption-last-month"],
	["twoMonthsAgoConsumption", "T#report.consumption-two-months-ago"],
	["expiringInSixMonths", "T#report.expiring-6-months"],
	["expiringInTwelveMonths", "T#report.expiring-12-months"],
	["stockOnOrder", "T#report.stock-on-order"],
	["AMC12", "T#report.amc-12-months"],
	["AMC24", "T#report.amc-24-months"]
	]
	}

7093 add sort toggle to prevent report filter dropdowns overlapping #7406

7093 add sort toggle to prevent report filter dropdowns overlapping #7406

Conversation

aimee-mcneil-melville commented Apr 17, 2025 • edited Loading

👩🏻‍💻 What does this PR do?

💌 Any notes for the reviewer?

🧪 Testing

📃 Documentation

📃 Reviewer Checklist

github-actions bot commented Apr 17, 2025 • edited Loading

Bundle size difference

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Petty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimee-mcneil-melville commented Apr 28, 2025

Chris-Petty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimee-mcneil-melville commented Apr 17, 2025 •

edited

Loading

github-actions bot commented Apr 17, 2025 •

edited

Loading