Skip to content
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

Improve Calendar form field style - Issues #36933 & #33447 #40761

Merged
merged 11 commits into from
Sep 8, 2023

Conversation

cyrezdev
Copy link
Contributor

@cyrezdev cyrezdev commented Jun 13, 2023

Pull Request for Issues #36933 & #33447.

Summary of Changes

Improve display of datetime picker.

  • Centering calendar table for better looking in some not-english language.
  • Style time select fields.
  • Fix footer buttons alignment.
  • Improve display of time fields depending on field settings (weeknumbers, am/pm).

Testing Instructions

  • Apply patch, and test any form field of type calendar.

Actual result BEFORE applying this Pull Request

before-patch-fr
Example in French

Expected result AFTER applying this Pull Request

after-patch-fr
Example in French

Rendering depending on calendar field settings:

With or without weeknumbers, am/pm and or time. And with RTL support

Legend:

  1. Week numbers, time 24h
  2. Week numbers, time 12h
  3. Time 24h
  4. Time 12h
  5. No Time
  6. RTL (arabic)

(1) weeknumber-time-24h (2) weeknumber-time-12h
(3) time-24h (4) time-12h
(5) weeknumbers-notime (6) rtl-support

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-4.3-dev labels Jun 13, 2023
@brianteeman
Copy link
Contributor

I am confused. If I understand correctly then the problem only exists in non-english languages where the text for "today" is long.
If that is correct then what is the purpose of all the screenshots you have posted in english

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Jun 13, 2023

I am confused. If I understand correctly then the problem only exists in non-english languages where the text for "today" is long. If that is correct then what is the purpose of all the screenshots you have posted in english

As mentionned in the description of this PR, it's not only for this point, but 3 other ones:

  • To style time select fields.
  • Fix footer buttons alignment.
  • Improve display of time fields depending on field settings (weeknumbers, am/pm).

So the additionnal screenshots are valid to show the different possible settings for calendar form field.

It is mainly cosmetic here and to have a more flexible design ;-)

@Chaosxmk
Copy link

Looks good, fixes the main point of issue I highlighted.

@obuisard
Copy link
Contributor

Looks good, fixes the main point of issue I highlighted.

@Chaosxmk, please help test the PR, we need two successful tests to get this great improvement merged into the project :-)

@obuisard obuisard changed the base branch from 4.3-dev to 4.4-dev August 14, 2023 00:32
@laoneo laoneo self-assigned this Aug 17, 2023
@laoneo laoneo changed the title [4.3] Improve Calendar form field style - Issues #36933 & #33447 Improve Calendar form field style - Issues #36933 & #33447 Sep 8, 2023
@drmenzelit
Copy link
Contributor

I have tested this item ✅ successfully on b668fc2

Tested with English and Persian


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40761.

@obuisard
Copy link
Contributor

obuisard commented Sep 8, 2023

I have tested this item ✅ successfully on 145606e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40761.

@obuisard
Copy link
Contributor

obuisard commented Sep 8, 2023

Viviana @drmenzelit, can you put your results in the issue tracker once more? I am sorry, the branch update invalidated your results. Thanks!

@obuisard
Copy link
Contributor

obuisard commented Sep 8, 2023

Great job Cyril @cyrezdev, much needed improvements and now we have a decent calendar field :-)

@laoneo laoneo merged commit da378aa into joomla:4.4-dev Sep 8, 2023
3 of 4 checks passed
@laoneo
Copy link
Member

laoneo commented Sep 8, 2023

Thanks! Also for the fast tests...

@laoneo laoneo added this to the Joomla! 4.4.0 milestone Sep 8, 2023
@cyrezdev
Copy link
Contributor Author

Great job Cyril @cyrezdev, much needed improvements and now we have a decent calendar field :-)

Thank you Olivier!

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Oct 20, 2023

Hello here!

This PR has introduced an issue with no possibility to set Time with calendar picker, if the week numbers are hidden and the time format is 24h.

Here a quick PR with the patch: #42185

Thank you for testing! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants