feat(widgets): refine bikram sambat calendar user experience (#9250) by megha1807 · Pull Request #11234 · medic/cht-core

megha1807 · 2026-07-02T03:13:24Z

Description

This PR implements three UX refinements to the Bikram Sambat calendar widget to improve user experience in Enketo forms, utilizing the shared datepicker module.

Key Changes

Keep Calendar Open on Date Selection: Configured the picker (closeOnDateSelect: false) and removed automatic hide triggers so that selecting a date keeps the calendar open.
Clear Option (Eraser Button): Added an eraser-shaped clear button next to the Today button inside the calendar header. Clicking it resets all manual fields, dropdown labels, hidden input, and the backing real date input.
Floating Close Button: Redesigned the close button (×) to float as a round circle with a drop shadow at the top-right corner (top: -12px; right: -12px) of the modal container.
Unit Tests: Added Karma test coverage in bikram-sambat-datepicker.spec.ts to verify the clear button behavior and ensure selection does not close the picker.

Issue Number

Fixes #9250

Code review checklist

Readable: Concise, well named, follows the style guide
Tested: Unit and/or e2e where appropriate
Internationalised: All user facing text
Backwards compatible: Works with existing data and configuration

)

sugat009

Thanks @megha1807, this is a clean extraction of the shared picker module and the three refinements work well. I pulled the branch and tested on a form with three bikram-sambat date fields.

Working as intended: keep-open on select, Esc and overlay dismiss, the eraser resetting its own field, the floating close button at normal window sizes, and the saved value staying correct.

Inline notes are below. Two are worth resolving before merge (the eraser not appearing on additional date fields, and the shared closeOnDateSelect value that collides with #11230). The rest are smaller, and a couple are questions rather than change requests.

sugat009 · 2026-07-06T09:15:40Z

+      $('<div class="nepali-date-picker-overlay"></div>').appendTo('body').show();
+    }
+
+    const container = document.querySelector('.nepali-date-picker');


issue (blocking): Eraser only appears on the first date field when a form has more than one.

Reproduced on a form with three bikram-sambat date fields: the eraser shows only on Date 1. document.querySelector('.nepali-date-picker') returns the first picker div in the DOM, and ensureClearButton returns early once any .clear-btn already exists (line 45), so the eraser lands in the first field's calendar only and the dedup guard then blocks every other field. The nepali-date-picker library appends a separate picker div per date field, so fields 2..N never get one.

Nepali forms often carry several date questions (DOB, registration, visit), so this affects the feature's main use case. Could we resolve the picker for the specific $hiddenInput being shown, then inject and bind the eraser per active picker, rather than querySelector first-match?

Suggested fix (one change in the shared module, also resolves the cleanupNepaliDatePicker finding on #11230):

Root cause: the module locates the picker with global / first-match selectors, but the library appends a separate .nepali-date-picker div per input. Scope every operation to the input's own div.

Capture the div the library creates right after .nepaliDatePicker() (it appends to <body> synchronously, so it is the last one) and stash it on the input:

$hiddenInput.nepaliDatePicker({ dateFormat: '%y-%m-%d', closeOnDateSelect: false }); const $picker = $('.nepali-date-picker').last(); // this input's own div $hiddenInput.data('picker', $picker);

Then use $picker everywhere a global selector is used today:

show handler: const container = $picker[0]; instead of document.querySelector('.nepali-date-picker'), so ensureClearButton and the MutationObserver bind to the correct picker (this is what fixes the eraser missing on fields 2 and 3).

close button: $picker.find('.close-btn') / $picker.append($closeButton).

hideDatePicker: $picker.hide() instead of $('.nepali-date-picker').hide().

On feat(#9699): display nepali (bikram sambat) calendar in reports filter #11230, cleanupNepaliDatePicker removes $hiddenInput.data('picker') rather than $('.nepali-date-picker').remove(), so the from/to filters can no longer delete each other's calendar.

The .nepali-date-picker-overlay backdrop is genuinely shared (a single element), so keeping it global is fine as long as only one picker is open at a time (true for both the form and the reports from/to). .last() relies on the library appending synchronously (it does); to be defensive you could diff the .nepali-date-picker set before and after the .nepaliDatePicker() call instead.

One fix here covers both the eraser bug on this PR and the cleanup fragility on #11230.

sugat009 · 2026-07-06T09:15:40Z

+
+  $hiddenInput.nepaliDatePicker({
+    dateFormat: '%y-%m-%d',
+    closeOnDateSelect: false


question (blocking): closeOnDateSelect: false is hardcoded in the shared module, which collides with #11230.

Setting closeOnDateSelect: false and dropping the auto-hide is right for the Enketo widget. The catch is that #11230 (#9699, Reports filter) creates this same file with closeOnDateSelect: true and relies on auto-close on select (its date-filter.component.ts consumer passes only onDateSelect and never closes the picker itself). Both PRs add this file, so whichever merges second silently flips the other surface's behavior.

Could we thread closeOnDateSelect (or a hideOnSelect) through the options object so the widget opts into keep-open while the Reports filter keeps auto-close? Since keeping the calendar open after selection is still awaiting @binokaryg's input, it may also be worth confirming that before we settle the default here.

I have resolved this by threading the closeOnDateSelect option into the options parameter of setupNepaliDatePicker (defaulting to true).

In this PR (#11234), the Enketo widget overrides it by passing closeOnDateSelect: false to keep the calendar open. In the other PR (#11230 / Reports Filter), it will default to true (auto-close on select), which prevents any collision regardless of which PR is merged first.

sugat009 · 2026-07-06T09:15:40Z

-      });
-      $('.nepali-date-picker').append($closeButton);
+    },
+    onClear: () => {


question (non-blocking): A cleared date stays visually highlighted when the calendar is reopened.

Tested: select असार १, clear via the eraser (day, month, and year all empty correctly), close, then reopen the same field. असार १ is still highlighted. The persisted value is correct, it is the library's internal selection that onClear does not reset. Cosmetic only. Worth resetting the library selection here if it is cheap, otherwise fine to note and defer.

I have fixed this. When clearing the date, we now look up the specific calendar container and remove the .active class from the table cells ($(container).find('td.active').removeClass('active')).

Reopening the picker after clicking the eraser now correctly displays the calendar without any previously selected date highlighted.

sugat009 · 2026-07-06T09:15:40Z

+    $('.calendar-btn').click();
+
+    const activeDays = $('.nepali-date-picker table tbody td.current-month-date:not(.disable)');
+    if (activeDays.length > 0) {


issue (non-blocking): This test can pass without ever selecting a date.

The day click is guarded by if (activeDays.length > 0) and the test then only asserts the picker is visible. If no active day renders in the test DOM, no click happens and the assertion reduces to "visible after open", which does not prove the keep-open-on-select behavior. Could we assert that a day was actually clicked (or fail when none is found) so it exercises the real path?

sugat009 · 2026-07-06T09:15:40Z

+    }
+  });
+
+  $hiddenInput.on('show', function() {


suggestion (non-blocking): The popup behaves as a modal but has no dialog role or focus management.

Tested: Esc closes the calendar (good), but Tab moves through the form fields behind the open calendar rather than staying inside it. The overlay blocks the mouse but not the keyboard, and the keep-open change makes this more noticeable since the calendar lingers after selection. A role="dialog", moving focus into the popup on open, and trapping focus would close the gap. Some of this predates the PR, so a follow-up is reasonable if preferred.

sugat009 · 2026-07-06T09:15:40Z


+  .clear-btn {
+    display: inline-block;
+    width: 16px;


suggestion (non-blocking): The eraser touch target is 16x16px.

That is below the roughly 24x24 minimum (WCAG 2.5.8) and smaller than the sibling controls, so it is easy to miss on the Android app. Padding the hit area to about 24x24 while keeping the 16px icon would help.

sugat009 · 2026-07-06T09:15:40Z

+  if (!$today.length || $(container).find('.clear-btn').length) {
+    return;
+  }
+  const $clear = $('<a href="#" class="clear-btn" title="मिति हटाउनुहोस्"></a>');


nitpick (if-minor): The eraser is an <a href="#"> performing an action.

A <button type="button"> with an aria-label would match the close and calendar buttons (activates on Enter and Space, correct button role). It does sit among the library's own anchor controls, so this is minor polish rather than a real inconsistency.

sugat009 · 2026-07-06T09:15:40Z

  opacity: 1 !important;
  pointer-events: auto !important;

+  overflow: visible !important; /* so the corner-floated button is not clipped */


nitpick: overflow: visible !important looks redundant.

Nothing sets overflow on .nepali-date-picker (the base rule and the library CSS both leave it at the default visible), and it cannot affect the month and year scroll lists since those live on a separate descendant and overflow is not inherited. Safe to drop the !important, or the whole line, unless it is meant as defensive documentation.

sugat009 · 2026-07-06T09:15:40Z

+      $parent.find('input[name="day"]').val('').trigger('change');
+      $parent.find('input[name="month"]').val('').trigger('change');
+      $parent.find('input[name="year"]').val('').trigger('change').trigger('blur');
+      $group.find('.month-dropdown button').html('महिना <span class="caret"></span>');


nitpick: महिना is hardcoded here and in the form TEMPLATE.

The reset label duplicates the placeholder defined in the TEMPLATE. A shared constant would keep the two in sync.

sugat009

LGTM. I re-reviewed the reworked code and verified the widget in-app on a form with three bikram-sambat date fields: the eraser now appears on all three fields (the multi-field bug is fixed), keep-open on select works, selecting a date populates the day/month/year and the underlying date input correctly, and the eraser clears the field and closes the picker. All of my earlier findings are addressed. Approving.

Two optional, non-blocking a11y polish items for a follow-up if you want them (not gating this PR):

webapp/src/css/bikramsambat.less .clear-btn sets outline: none with no :focus-visible replacement, so keyboard focus on the eraser has no visible indicator (and it is inconsistent with the close button, which keeps its ring).
The focus trap in bikram-sambat-picker-shared.js can be escaped: the library rebuilds the calendar on month-navigation/selection, which destroys the focused control, focus falls back to <body>, and the container-scoped trap stops firing. Re-focusing the container after a re-render would close that gap.

Thanks for the thorough rework, @megha1807.

sugat009 · 2026-07-07T12:29:16Z

+  $(container).find('a').each(function() {
+    const $a = $(this);
+    const href = $a.attr('href') || '';
+    if (href.startsWith('javascript:')) {


suggestion (non-blocking): complete the URL scheme check to also cover data: and vbscript:.

CodeQL flags this as an "Incomplete URL scheme check" because it only neutralizes javascript:. It is not exploitable here (this runs only over the library-generated calendar, whose only anchors are javascript:void(0) nav buttons, so no untrusted input reaches it), so it is hardening rather than a live vuln. But it is the alert currently keeping the CodeQL check red on the epic PR #11211, and completing the check clears it once this merges into the feature branch.

Suggested change

if (href.startsWith('javascript:')) {

if (/^\s*(javascript|data|vbscript):/i.test(href)) {

Heads-up: the identical sanitizeHrefs in #11230's copy of this shared module needs the same one-liner, so whichever of the two merges into the feature branch first should carry it (or both), otherwise the alert reappears.

Thanks for the suggestion! I've updated the URL scheme check to use the recommended regex (/^\s*(javascript|data|vbscript):/i.test(href)) in the shared helper across both PRs (#11234 and #11230).

Whichever merges first will carry the fix and clear the CodeQL alert!

… validation

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch from 2fedf90 to 91f802d Compare July 2, 2026 03:52

feat(widgets): refine bikram sambat calendar user experience (medic#9250

369bd3a

)

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch from 91f802d to 369bd3a Compare July 2, 2026 03:58

sugat009 self-requested a review July 3, 2026 06:49

sugat009 assigned megha1807 Jul 3, 2026

sugat009 requested changes Jul 6, 2026

View reviewed changes

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch 4 times, most recently from 3df0dc3 to 5ca3e29 Compare July 6, 2026 12:12

megha1807 requested a review from sugat009 July 6, 2026 12:51

sugat009 mentioned this pull request Jul 6, 2026

feat(#9699): display nepali (bikram sambat) calendar in reports filter #11230

Merged

4 tasks

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch 2 times, most recently from ddf4670 to f7120ef Compare July 6, 2026 16:22

chore(ci): trigger build

02ecc3c

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch from f7120ef to 02ecc3c Compare July 6, 2026 16:27

sugat009 approved these changes Jul 7, 2026

View reviewed changes

sugat009 mentioned this pull request Jul 7, 2026

feat(#10707): enhance Bikram Sambat support in CHT #11211

Draft

7 tasks

sugat009 reviewed Jul 7, 2026

View reviewed changes

megha1807 added 2 commits July 7, 2026 19:55

Merge target branch updates

5635bc5

feat(widgets): merge target updates and implement CodeQL scheme check…

d7b9c5e

… validation

megha1807 force-pushed the fix/9250-bs-datepicker-refinements branch from 231a328 to d7b9c5e Compare July 7, 2026 14:38

sugat009 merged commit ee35d69 into medic:10707-dmp-2026-enhance-bikram-sambat-support-in-cht Jul 8, 2026
27 of 28 checks passed

sugat009 linked an issue Jul 8, 2026 that may be closed by this pull request

Improve Bikram Sambat date picker #9250

Open

This was referenced Jul 8, 2026

Improve Bikram Sambat date picker #9250

Open

Bikram Sambat enketo widget leaks body-appended picker DOM on each form open (destroy() never runs) #11245

Closed

[DMP 2026]: Enhance Bikram Sambat support in CHT #10707

Open

	if (href.startsWith('javascript:')) {
	if (/^\s*(javascript\|data\|vbscript):/i.test(href)) {

Uh oh!

Uh oh!

Conversation

megha1807 commented Jul 2, 2026

Description

Key Changes

Issue Number

Code review checklist

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants