User tests: Successful: Unsuccessful:
Pull Request for Issue #45082
Fixed the issue where the selected day in the frontend calendar was not highlighted.
described in #45082
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 |
| Category | ⇒ | Repository NPM Change |
@krishnaGandhi11 Please fix the code style errors reported here: https://github.com/joomla/joomla-cms/actions/runs/21568209712/job/62143186648?pr=46811
| Labels |
Added:
NPM Resource Changed
PR-5.4-dev
|
||
@krishnaGandhi11 The pull request shall not include the compiled css results
media/templates/site/cassiopeia/css/template.cssandmedia/templates/site/cassiopeia/css/template.min.css.
Sorry about that! I have removed it now.
@krishnaGandhi11 There are still code style errors reported in the CI checks: https://github.com/joomla/joomla-cms/actions/runs/21568431305/job/62143634597?pr=46811
| Labels |
Added:
bug
|
||
@krishnaGandhi11 There are still code style errors reported in the CI checks: https://github.com/joomla/joomla-cms/actions/runs/21568431305/job/62143634597?pr=46811
Thanks @richard67
So the question isnt - does the css in this PR address the problem but more why doesnt the css in calendar.css work in the front end bootstrap based site template when it works in the bootstrap based admin template. When you identify that then surely that is what should be fixed and not new css added
So the question isnt - does the css in this PR address the problem but more why doesnt the css in calendar.css work in the front end bootstrap based site template when it works in the bootstrap based admin template. When you identify that then surely that is what should be fixed and not new css added
Backend template (Atum) works because it includes a specific _calendar.scss file that maps the system variables (like --btn-primary-bg) to the template colors. Cassiopeia is missing this mapping entirely.
Instead of adding custom CSS overrides, I tried adding the missing variable definitions to the :root block in template.scss.
This "activates" the existing core styles for the frontend.
it works - the front end calendar behaves as expected.
does this align with your question ?
so you are getting close. look at how and where this is done in the atum template
I have tested this item ✅ successfully on 30ad40e
// Calendar Highlighting in Frontend
.js-calendar .calendar-container tbody td.day.selected {
background-color: var(--cassiopeia-color-primary) !important;
border: 1px solid var(--cassiopeia-color-primary) !important;
color: #fff !important;
border-radius: 4px !important;
}
// Hover Effect
.js-calendar .calendar-container tbody td.day:not(.disabled):not(.othermonth):hover {
background-color: var(--cassiopeia-color-primary) !important;
color: #fff !important;
cursor: pointer;
}issue solved
I have tested this item ✅ successfully on 30ad40e// Calendar Highlighting in Frontend .js-calendar .calendar-container tbody td.day.selected { background-color: var(--cassiopeia-color-primary) !important; border: 1px solid var(--cassiopeia-color-primary) !important; color: #fff !important; border-radius: 4px !important; }
// Hover Effect .js-calendar .calendar-container tbody td.day:not(.disabled):not(.othermonth):hover { background-color: var(--cassiopeia-color-primary) !important; color: #fff !important; cursor: pointer; }issue solved
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46811.
Although I have changed the way of solving this issue and just pushed the updated fix,
The code snippet you pasted above includes !important flags, but my previous version/fix(which you tested) never used "!important" - I don't understand why you commented that, is that a suggestion ?
@465645 Could you test this PR again with the latest changes, and also answer @krishnagandhicode question regarding the CSS code in your previous test result? That would be great, thanks in advance.
The code snippet with !important flags was mentioned as an alternative method to achieve the same calendar highlighting effect
I have tested this item ✅ successfully on 196c95f
whole code is working properly but, I've encounter one issue i.e. The CSS selectors in the calendar highlighting code were missing the tbody td elements in the selector path. corrected code snippet is
.js-calendar .calendar-container tbody td.day.selected
.js-calendar .calendar-container tbody td.day:not(.disabled):not(.othermonth):hover
I have tested this item ✅ successfully on 196c95fwhole code is working properly but, I've encounter one issue i.e. The CSS selectors in the calendar highlighting code were missing the tbody td elements in the selector path. corrected code snippet is .js-calendar .calendar-container tbody td.day.selected .js-calendar .calendar-container tbody td.day:not(.disabled):not(.othermonth):hover
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46811.
To clarify: This PR solely focuses on defining the missing CSS variables (--btn-primary-bg, --btn-primary-color) that were causing the issue. (you must verify here).
I have tested this item ✅ successfully on 196c95f
I am still not convinced that the proposed solution is correct. Seems wrong to me to be setting a root variable in this file. I woujld like to hear from some of the authors of the cassiopeia template to understand why this root variable was missing and if this is the correct place to put it back
It is the wrong place. A calendar is not a block of the template like banner or footer.
It is the wrong place. A calendar is not a block of the template like banner or footer.
@chmst Any suggestion where else it should go? Or should we wait for @drmenzelit 's review?
| Labels |
Added:
Updates Requested
|
||
I have tested this item ✅ successfully on 3ac67ed
I have tested this item ✅ successfully on 3ac67ed
It works for me.
I have tested this item ✅ successfully on 3ac67ed
| Status | Pending | ⇒ | Ready to Commit |
RTC
I have tested this item ✅ successfully on 3ac67ed
I have tested this successfully. Thank @krishnaGandhi11 and congrats!
I have tested this item ✅ successfully on 3ac67edI have tested this successfully. Thank @krishnaGandhi11 and congrats!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46811.
@exlemor Thanks a lot! It feels good to see my first contributions making it in : )
@drmenzelit As fas as I can see, the changes which you have requested have been implemented. Can you confirm that?
@richard67 yes, the requested changes are done, that is why I had removed the label.
@richard67 yes, the requested changes are done, that is why I had removed the label.
@drmenzelit But you haven’t approved in GitHub review, so Gitahib still shows the unresolved conversation. I will dismiss that for you.
@richard67 github drives me crazy ;-)
Do we need new tests?
| Labels |
Added:
RTC
Removed: Updates Requested |
||
I've just done a final test for this PR here before I wanted to merge it.
For LTR it works as described and fixes the issue.
For RTL this PR does nothing. Highlighting the selected day and hover colours work for RTL in the same way without and with this PR, using different colours than LTR which are not changed by this PR.
@brianteeman What do you think? Can we live with that as this PR does not change anything, LTR and RTL were different without this PR and still are with this PR? Or should that be fixed? If to be fixed, should it done with this PR here or a separate one?
I would prefer to leave it as it is or fix it with a separate, new PR.
I mentioned RTL in my comments ... I forgot about that later...
@drmenzelit Could that be done with a follow-up PR? To me it seems RTL and LTR were also different in past.
@krishnagandhicode Would you make such follow up PR for RTL?
P.S.: We could create a new issue for the RTL calendar thing so we don't forget about it.
@richard67 Agree with the approach-I'll handle the RTL fix in a separate follow-up PR as suggested.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2026-02-17 19:24:36 |
| Closed_By | ⇒ | richard67 |
Thanks @krishnagandhicode for this PR, and thanks to all testers.
@richard67 Agree with the approach-I'll handle the RTL fix in a separate follow-up PR as suggested.
@krishnagandhicode See issue #46904 for the follow up PR for RTL.
@krishnagandhicode Please wait a bit with a follow up PR. It might be that there is a better solution that adding stuff for RTL, which would be to re,ove the extra rtl css. file. See discussion in issue #46904 .
@krishnagandhicode Please wait a bit with a follow up PR. It might be that there is a better solution that adding stuff for RTL, which would be to re,ove the extra rtl css. file. See discussion in issue #46904 .
Yeah I am reading through it.
@krishnaGandhi11 The pull request shall not include the compiled css results
media/templates/site/cassiopeia/css/template.cssandmedia/templates/site/cassiopeia/css/template.min.css.