Skip to content

FIX calendar picker if weeknumbers hidden and time format 24h#42183

Closed
cyrez wants to merge 1 commit intojoomla:5.0-devfrom
cyrez:patch-55
Closed

FIX calendar picker if weeknumbers hidden and time format 24h#42183
cyrez wants to merge 1 commit intojoomla:5.0-devfrom
cyrez:patch-55

Conversation

@cyrez
Copy link
Contributor

@cyrez cyrez commented Oct 20, 2023

Pull Request for Issue introduced (by me 🥇 ) in PR #40761.

Affected Joomla versions: 4.4.0 and 5.0.0

Summary of Changes

Quick FIX
Check if weeknumbers is true or false to listen to correct number of childNodes.
Calendar form field with 12h (AM/PM) time format is not affected by the issue, as well as calendar picker with week numbers (so Joomla core not affected).

Testing Instructions

Set 2 form fields of type calendar like this (with and without week numbers):

Note: default timeformat is 24 and dedault weeknumbers is true.

<field
	name="no_week_numbers"
	type="calendar"
	label="Test"
	description="Test without weeknumbers and time 24h"
	showtime="true"
	weeknumbers="false"
/>

<field
	name="week_numbers"
	type="calendar"
	label="Test"
	description="Test with weeknumbers and time 24h"
	showtime="true"
/>

Actual result BEFORE applying this Pull Request

You can't change Time (hour and/or minutes) with the no_week_numbers calendar picker.
It's working with the other form field.

Expected result AFTER applying this Pull Request

It's working in both cases.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Oct 20, 2023
@richard67
Copy link
Member

richard67 commented Oct 20, 2023

As it is a bug fix which is also relevant for 4.4, this pull request should be rebased or redone for the 4.4-dev branch. Maintainers will merge it up to 5.0-dev when it has been merged in 4.4-dev.

@richard67 richard67 added the bug label Oct 20, 2023
@cyrez
Copy link
Contributor Author

cyrez commented Oct 20, 2023

Thanks @richard67 !

I will close this PR in favor of this new one: #42185

@cyrez cyrez closed this Oct 20, 2023
@richard67
Copy link
Member

Thanks to you for the PR, @cyrezdev .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants