Skip to content

Use new item list implementation #315

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

sukhwinder33445
Copy link
Contributor

No description provided.

@sukhwinder33445 sukhwinder33445 self-assigned this Apr 2, 2025
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Apr 2, 2025
@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch 6 times, most recently from 2e6714c to 7c984c9 Compare April 9, 2025 10:51
@sukhwinder33445 sukhwinder33445 requested a review from nilmerg April 9, 2025 11:16
@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch from 21d099e to cb4d748 Compare April 10, 2025 07:13
{
public function assembleAttributes($item, Attributes $attributes, string $layout): void
{
$attributes->get('class')->addValue('schedule-item');
Copy link
Member

Choose a reason for hiding this comment

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

schedule please. None of the other renderers use -item as suffix either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The class schedule is already reserved by the Widget/Schedule. I didn't want to make any changes to Widget/Schedule class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a comment here that the suffix is required because the css class is already taken.
OR
Should I change the name of the css in Widget/Schedule to schedule-widget?

Copy link
Member

Choose a reason for hiding this comment

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

Funny, the ScheduleController already aliases it to ScheduleWidget. I guess now is the time to rename that thing entirely: Icinga\Module\Notifications\Widget\Detail\ScheduleDetail

@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch from cb4d748 to 42dc33f Compare April 17, 2025 14:44
@jrauh01 jrauh01 mentioned this pull request Apr 24, 2025
@jrauh01
Copy link
Contributor

jrauh01 commented Apr 24, 2025

Fixes #330

- Links: Add missing channel links
- Remove obsolete code
- Links: Add missing contactAdd() method
- Remove obsolete code
- Remove obsolete code
- Remove obsolete code
@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch 2 times, most recently from 6e5c73f to 75ac72a Compare April 25, 2025 12:26
- Remove obsolete code
- Adjust Schedule-list.less accordingly
- Remove obsolete code
- Remove obsolete code
- ObjectList: Add method to disable action-list functionality
- Remove obsoltete code
This padding is now handled by ipl-web

- Optimize code
@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch from 75ac72a to decc109 Compare April 25, 2025 12:40
@sukhwinder33445 sukhwinder33445 force-pushed the use-new-item-list-implementation branch 2 times, most recently from eaebc58 to b0ff808 Compare April 25, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants