Skip to content

Comments

Formatted the calendar widget#36995

Closed
akshitrattan wants to merge 10 commits intojoomla:4.3-devfrom
akshitrattan:Fix#36933
Closed

Formatted the calendar widget#36995
akshitrattan wants to merge 10 commits intojoomla:4.3-devfrom
akshitrattan:Fix#36933

Conversation

@akshitrattan
Copy link

Pull Request for Issue #36933 .

Summary of Changes

solved issue number 3/3.
Stylized the time drop-down menu according to bootstrap 4 and centred the calendar table in the widget for the calendar to look more symmetrical

Testing Instructions

Open the calendar widget in any of the tabs you wish to, notice the change in the buttons from the previous versions

Actual result BEFORE applying this Pull Request

calendar looked like this
Screenshot 2022-02-10 at 7 13 25 PM

Expected result AFTER applying this Pull Request

it should look like this
Screenshot 2022-02-10 at 7 13 48 PM

Documentation Changes Required

No changes required

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.1-dev labels Feb 10, 2022
@akshitrattan akshitrattan changed the title formatted the calendar widget Formatted the calendar widget Feb 10, 2022
.buttons-wrapper .btn:last-child {
margin-right: 0;
}
/* .time {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not commit commented out code1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry about that, I made the changes

z-index: 1100 !important;
}
.calendar-container table {
margin: 0px 0px 0px 55px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the value is 0 then you should not put any units

Suggested change
margin: 0px 0px 0px 55px;
margin: 0 0 0 55px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be better to use a margin: 0 auto; instead of setting a hard amount?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be better to use a margin: 0 auto; instead of setting a hard amount?

there is an empty td element in the entire 'time' row so setting the attribute to auto won't work

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how the empty td element would be affecting the alignment of the table itself, that td should only be affecting that row. When I submitted the original issue, I noted all of the inline edits I made and margin: 0 auto; on the table had no negative impact when tested. Granted, I do not know if how to script builds the popup+table would be affected by auto, so if something there does affect it, ignore my suggestion.

@HLeithner HLeithner changed the base branch from 4.1-dev to 4.2-dev June 27, 2022 13:06
@HLeithner
Copy link
Member

This pull request has automatically rebased to 4.2-dev.

@joomla-bot
Copy link
Contributor

This pull requests has been automatically converted to the PSR-12 coding standard.

@chmst
Copy link
Contributor

chmst commented Oct 24, 2022

I have tested this item ✅ successfully on 259bc23

Looks good for me. Especially in languages with longer words.


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

@Hackwar Hackwar added the Small A PR which only has a small change label Feb 25, 2023
@Hackwar Hackwar added the bug label Apr 6, 2023
@HLeithner HLeithner changed the base branch from 4.2-dev to 4.3-dev May 2, 2023 16:30
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 4.3-dev.

@MojoMojosoup
Copy link

I have tested this item ✅ successfully on cfef221


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

@alikon alikon removed the PR-4.2-dev label May 22, 2023
@cyrez
Copy link
Contributor

cyrez commented Jun 13, 2023

I have tested this item 🔴 unsuccessfully on cfef221

Just find your PR, while i've done one to improve calendar form field datetime picker #40761

I've set my test as unsuccessfully, as the colspan changes may have issues depending on the form field settings (with or without weeknumbers, am/pm...)
And the css changes are not flexible to work in different cases (language, settings...).

You can check my PR #40761 and see what i did there (with a bit more styling for other elements, such as footer buttons) ;-)

Thanks!
Cyril


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

@obuisard
Copy link
Contributor

Thank you Akshit @akshitrattan for this PR. After review, I think this is not flexible enough and results may vary depending on the languages used. I am closing the PR in favor of PR #40761.
Thank you for your contribution on this!

@obuisard obuisard closed this Jun 20, 2023
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 Small A PR which only has a small change

Projects

None yet

Development

Successfully merging this pull request may close these issues.