[#45119] - [5.3] Calendar: Highlight selected day in frontend.
- Pending
- Medium
- Build: 5.2-dev
- # 45119
- Diff
- krishnaGandhi11:feature
User tests: Successful: Unsuccessful:
This is a reopened version of PR #45096
I am unable to reopen #45096 because of technical reasons, sorry ;)
Testing Instructions
-Open an article and try to set a publishing date (this opens the calendar).
-Select a date and check if it gets highlighted properly.
-Test on multiple browsers (Chrome, Firefox, Edge, Safari).
Actual result BEFORE applying this Pull Request
-The selected date in the calendar was not highlighted in the frontend.
-The backend calendar worked correctly, but the frontend did not.
Expected result AFTER applying this Pull Request
-The selected date should now be properly highlighted in the frontend calendar, just like in the backend.
video-sr.mp4
krishnaGandhi11
krishnaGandhi11 commented 4 days ago
Pull Request for Issue #45082
Summary of Changes :
-Fixed the issue where the selected day in the calendar was not highlighted in the frontend.
-Applied correct styling to match the backend calendar appearance.
Testing Instructions :
-Open an article and try to set a publishing date (this opens the calendar).
-Select a date and check if it gets highlighted properly.
-Test on multiple browsers (Chrome, Firefox, Edge, Safari).
Actual result BEFORE applying this Pull Request :
-The selected date in the calendar was not highlighted in the frontend.
-The backend calendar worked correctly, but the frontend did not.
Before
Expected result AFTER applying this Pull Request :
-The selected date should now be properly highlighted in the frontend calendar, just like in the backend.
Fixed.mp4
Fixes #45082
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
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | NPM Change Front End Templates (site) |
| Labels |
Added:
NPM Resource Changed
PR-5.2-dev
|
||
For all the reasons I have already explained this PR is wrong AND you have committed unrelated files
AND you have committed unrelated files
@brianteeman It's only one file, the package-lock.json, and the only change in that file is the Joomla version change to 5.2.6, which happens as a result of npm ci or npm install. It would not do harm to have it in this PR, but of course it wouldbe cleaner to keep that separate.
@krishnaGandhi11 Could you revert the change in the package-lock.json file? Thanks in advance.
| Category | NPM Change Front End Templates (site) | ⇒ | Front End Templates (site) |
For all the reasons I have already explained this PR is wrong AND you have committed unrelated files
As I mentioned earlier, my last PR got deleted, so I raised the same one again to continue working on it further.
AND you have committed unrelated files
@brianteeman It's only one file, the package-lock.json, and the only change in that file is the Joomla version change to 5.2.6, which happens as a result of
npm ciornpm install. It would not do harm to have it in this PR, but of course it wouldbe cleaner to keep that separate.@krishnaGandhi11 Could you revert the change in the package-lock.json file? Thanks in advance.
@richard67 Thanks for pointing that out! I've now reverted the changes in package-lock.json to keep the PR clean. Appreciate your guidance!
I have tested this item ✅ successfully on 9574158
I have re-tested this successfully...
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45119.
This pull request has been automatically rebased to 5.3-dev.
| Title |
|
||||||
I have tested this item ✅ successfully on 9574158
I tested this on Edge, Firefox and Chrome, and it worked successfully
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45119.
We have to invalidate the last test, it was done from a Joomla Day USA instance, which does not allow the testing of NPM labelled PRs
Nice to see joomladay USA PBF testing :) we will check this later
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45119.
I fear that this has some effect on elements which do rely on this variable.
@krishnaGandhi11 thank you for your PR, but it is the wrong way to solve the problem. You shouldn't add variables directly in the index.php from Cassiopeia. As Allon wrote above, since these variables are part of Bootstrap, changing them in this way could have side effects on other elements using the class "btn-primary". And the colors should match with the colors defined in Cassiopeia.
| Status | Ready to Commit | ⇒ | Pending |
@krishnaGandhi11 thank you for your PR, but it is the wrong way to solve the problem. You shouldn't add variables directly in the index.php from Cassiopeia. As Allon wrote above, since these variables are part of Bootstrap, changing them in this way could have side effects on other elements using the class "btn-primary". And the colors should match with the colors defined in Cassiopeia.
@drmenzelit Thanks for the feedback! I'm aware of the issue but have still not figured out the exact way to fix this.
| Labels |
Added:
Updates Requested
bug
PR-5.3-dev
Removed: NPM Resource Changed PR-5.2-dev |
||
I have tested this item ✅ successfully on fa62ec3
I have tested this successfully. Thanks Krishna.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45119.