Skip to content

Remove dead no-parent-event prop from the date picker - #218

Closed
evnchn wants to merge 1 commit into
mainfrom
fix-date-input-dead-no-parent-event-prop
Closed

Remove dead no-parent-event prop from the date picker#218
evnchn wants to merge 1 commit into
mainfrom
fix-date-input-dead-no-parent-event-prop

Conversation

@evnchn

@evnchn evnchn commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Motivation

DateInput applies .props('no-parent-event') to the inner QDate picker. Quasar 2.18.5's QDate has no no-parent-event prop — noParentEvent belongs to the anchor-props mixin that backs QMenu/QTooltip/QPopupProxy, not QDate. So the prop is a dead no-op: Vue passes it through as an inert root DOM attribute that Quasar never reads.

Introduced in #4815 (commit b8b75b20). For contrast, the only place no-parent-event was ever load-bearing was on a QMenu (the 2023 83951acb "remove no-parent-event hack") — a different structural position from this child QDate.

Implementation

Drop .props('no-parent-event') from the QDate only. The wrapping QMenu correctly never carried it (the calendar button should open the menu on its parent click). No behavior change — open / close / select is driven by the parent QMenu, not this inert child attribute.

Evidence: Quasar bundle grep + independent review
  • In the vendored nicegui/static/quasar.umd.js (Quasar v2.18.5), noParentEvent is defined only inside the anchor-props mixin (consumed by configureAnchorEl). QDate's prop set (useDatetimeProps + useFormProps + useDarkProps + modelValue/multiple/range/…) does not include it.
  • ui.menu() does not set no-parent-event, so the button correctly opens the picker — removing the child's attribute can't change that.
  • Repo grep finds no NiceGUI .js/.vue/.css reading no-parent-event.
  • No test added: there is no behavioral delta to assert, and prop-presence assertions aren't the repo convention (only test_defaults/test_element_filter check prop values).
  • Different-lineage review (Codex, adversarial): approve / SHIP — "QDate in vendored Quasar 2.18.5 does not define noParentEvent … could only fall through as an inert root DOM attribute … open/close behavior is controlled by the parent QMenu. No material findings."

Local gates: pre-commit ✓ · mypy ./nicegui ✓ · pylint ./nicegui 10.00/10 ✓

Progress

  • The PR title is a short phrase starting with a verb.
  • The implementation is complete.
  • This PR does not address a security issue.
  • Pytests are not necessary (inert-attribute removal, no behavioral delta; prop-presence asserts are off-convention).
  • Documentation is not necessary.
  • No breaking changes to the public API.

@evnchn
evnchn force-pushed the fix-date-input-dead-no-parent-event-prop branch from 0996738 to c4b254c Compare July 24, 2026 09:16
@evnchn

evnchn commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author
  • We typically do not test these small details?
  • Who even introduced no-parent-event? Git blame time
  • Is it plausible and entertained that a dev uses no-parent-event for their own shenanigans? If it is, we have to remove in 4.0

@evnchn

evnchn commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

All three are fair — and one of them turns this from a "fix" into a judgement call that's yours to make.

1. "We typically do not test these small details?" — Correct, you don't. Grepping tests/, the only prop-level assertions are test_defaults / test_element_filter, and both check prop values / defaults, never "is this internal prop absent." A dead-attribute removal has no behavioral delta to assert, so the test I added is off-convention. I'd drop it and check the PR template's "Pytests … are not necessary" box. (Kept a fail-first test only to honour my own regression-test rule; happy to remove it.)

2. "Who even introduced no-parent-event? Git blame time." — 🫵 You did. git log -S pins it to commit b8b75b20, PR #4815 ("Feat: ui.date_input and ui.time_input", 2025-11-06). So this is self-inflicted, not inherited.

3. "Plausible a dev uses no-parent-event for their own shenanigans? → remove in 4.0" — the honest analysis

Where it sits: it's on DateInput's internal self.picker (date().props('no-parent-event')), not a user-facing parameter. There is no ui.date_input(no_parent_event=...) surface. So a user can't "use it" through the documented API — they'd have to reach into date_input.picker and rely on an inert DOM attribute that Quasar's QDate never reads.

Does anything depend on it today? No:

  • QDate has no noParentEvent prop (it's in the anchor mixin behind QMenu/QTooltip/QPopupProxy) → inert passthrough attribute.
  • The wrapping ui.menu() does not set no-parent-event, so the calendar button correctly opens the picker on parent click — removing the child's attribute can't change that.
  • For contrast, the only place no-parent-event was ever load-bearing was on a QMenu — the 83951acb "remove no-parent-event hack" (Falko, 2023) was ui.menu().props(remove='no-parent-event'), i.e. the anchor, a different structural position from this child QDate.

So the only "shenanigan" surface is: someone writing CSS/JS targeting q-date[no-parent-event] on the date-input picker specifically. Real, but vanishingly unlikely (internal, undocumented, inert). That's a walk-forward-never-backward DOM-observability call, not a functional one.

Your call — I'd frame it as three picks:

  • (a) Merge now as cleanup (drop the test): treat an inert, internal, undocumented attribute as fair game outside the major-version gate. My lean, given it's your own recently-added line and nothing reads it.
  • (b) Retarget 4.0: if the stance is "any DOM-attribute a user could observe is major-only," park it on the 4.0 breaking-changes list and close this PR until then.
  • (c) Drop it: if the churn isn't worth it, close — it's genuinely harmless as-is.

TL;DR: git-blame says it's yours (#4815); I'll drop the non-idiomatic test; and the merge-vs-4.0-vs-drop is a DOM-observability judgement I'll leave to you — (a) merge-as-cleanup is my lean, but I won't touch it further until you pick.

DateInput applied .props('no-parent-event') to the inner QDate, but
Quasar 2.18.5's QDate has no such prop (noParentEvent belongs to the
anchor-props mixin behind QMenu/QTooltip). It was an inert no-op
attribute; removing it changes no behavior (open/close is driven by the
parent QMenu).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@evnchn
evnchn force-pushed the fix-date-input-dead-no-parent-event-prop branch from c4b254c to c1cc06a Compare July 24, 2026 16:14
@evnchn

evnchn commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Per your call: merge as cleanup. Dropped the off-convention test (amended to a clean 1-line diff), body updated to "pytests not necessary". Marking ready + auto-merge on green.

@evnchn
evnchn marked this pull request as ready for review July 24, 2026 16:14
@evnchn

evnchn commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Correction: fork-merge brings no merit — the destination is upstream. Graduated → zauberzeug#6177. Closing this fork PR.

@evnchn evnchn closed this Jul 24, 2026
@evnchn
evnchn deleted the fix-date-input-dead-no-parent-event-prop branch July 25, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant