Nav refactoring

[Gerych1984]

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	✔️

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (develop@76d3a7a). Learn more about missing BASE report.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #157 +/- ## ========================================== Coverage ? 98.53% Complexity ? 536 ========================================== Files ? 27 Lines ? 1702 Branches ? 0 ========================================== Hits ? 1677 Misses ? 25 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[terabytesoftw]

Please let me know when it's ready for review.

[Gerych1984]

Please let me know when it's ready for review.

Already now, thx

[Gerych1984]

I've kept the old widgets for now, so I can peek around and not forget any functionality

[terabytesoftw]

It looks good at first, some observations.

• You can use enums for constants and avoid psalm-suppress, so the user can use them too.
• PSALM needs to be fixed.
• What is the difference between Menu::class and NavTabs::class, you couldn't have a single widget for both cases.
• TabPane::class what do we mean by this widget.
• You need to add phpdoc to all widgets.
• Move enums to a different directory than widgets, it could be Enum.
• Move abstract class to a diffirent directory than widgets, it could be Base.

[Gerych1984]

It looks good at first, some observations.

• You can use enums for constants and avoid psalm-suppress, so the user can use them too.
• PSALM needs to be fixed.
• What is the difference between Menu::class and NavTabs::class, you couldn't have a single widget for both cases.
• TabPane::class what do we mean by this widget.
• You need to add phpdoc to all widgets.
• Move enums to a different directory than widgets, it could be Enum.
• Move abstract class to a diffirent directory than widgets, it could be Base.

1. Yes, i think about this, but I didn't do the rough draft
2. I can't. Locally, I have a psalm configuration error
3. Menu (Nav in future) - just for a menu with links with no content. If you combine it with tabs, you don't need abstraction, but you need to mark the necessity to render content somehow
4. TabPane(l) - An auxiliary widget to indicate that a link/tab has content. TabPane always contains a link associated with it and they customize each other in this case. NavLink can be either a standalone tab link or content (in these cases the link attributes are not set).
5. We should probably get this to a satisfying conclusion first. ))
6. Yes

[terabytesoftw]

Yes, the code has improved a lot, some observations.

• Add phpdoc to classes and methods.

If you don't have any problems i can fix the static analysis and upload the changes, then send the PR for review and merge it, and continue improving this package.

[Gerych1984]

Yes, the code has improved a lot, some observations.

• Add phpdoc to classes and methods.

If you don't have any problems i can fix the static analysis and upload the changes, then send the PR for review and merge it, and continue improving this package.

Yes. Later i can show you my psalm config error. But i think - it last step. By the way - how do you think about move that or similar widgets/components (where few files) in sub-directory?

[terabytesoftw]

I think that widgets in the root directory are fine, because we have a shorter namespace, ideally everything that is not a widget can be moved, anyway let's ask for opinions from @samdark @vjik

[terabytesoftw]

If widgets can be used with other widgets, then of course i would rename them, one thing we want to do is replace the array items in all widgets with one widget.

[Gerych1984]

That my pslam error

[terabytesoftw]

It would be great if we convert NavItem::widget() to Item::widget(), and add support for icons, template to build in link as we wanted, support for container by link, with these changes Item::widget() would be agnostic, and could be used in all widgets that need a menu.

We could do it in a separate PR and thus eliminate the array methods from all the widgets.

[terabytesoftw]

That my pslam error

It looks like you either have a different version of psalm installed globally, or you have an outdated version of psalm that the package uses.

[Gerych1984]

That my pslam error

It looks like you either have a different version of psalm installed globally, or you have an outdated version of psalm that the package uses.

Maybe i have error with PHP 8.3

[Gerych1984]

That my pslam error

It looks like you either have a different version of psalm installed globally, or you have an outdated version of psalm that the package uses.

It would be great if we convert NavItem::widget() to Item::widget(), and add support for icons, template to build in link as we wanted, support for container by link, with these changes Item::widget() would be agnostic, and could be used in all widgets that need a menu.

We could do it in a separate PR and thus eliminate the array methods from all the widgets.

Maybe rename NavItem -> Item, NavLink -> Link. It strongly looks like this could be used in conjunction with Dropdown in future

Dropdown::widget()
     ->items(
             Link::widget(),
             Link::widget()->item(Item::widget())
)

Default Dropdown list can generally be inherited from AbstractNav

[terabytesoftw]

That my pslam error

It looks like you either have a different version of psalm installed globally, or you have an outdated version of psalm that the package uses.

It would be great if we convert NavItem::widget() to Item::widget(), and add support for icons, template to build in link as we wanted, support for container by link, with these changes Item::widget() would be agnostic, and could be used in all widgets that need a menu. We could do it in a separate PR and thus eliminate the array methods from all the widgets.

Maybe rename NavItem -> Item, NavLink -> Link. It strongly looks like this could be used in conjunction with Dropdown in future

Dropdown::widget()
     ->items(
             Link::widget(),
             Link::widget()->item(Item::widget())
)

Default Dropdown list can generally be inherited from AbstractNav

If we can use it in all the widgets that generate a menu, which are several, let's do it.

[Gerych1984]

@terabytesoftw can you look and help? Maybe remove Button widget and replace/rename it with Link

[terabytesoftw]

What is missing to finish the PR?

[Gerych1984]

What is missing to finish the PR?

It's works, so i need your solution ))

There are a few things I don't like related to immutability. But as a prototype, it's a 100% workable option

[terabytesoftw]

I'm thinking of merging this PR into a develop branch, so that the missing stuff can be addressed, do you agree?

[Gerych1984]

I'm thinking of merging this PR into a develop branch, so that the missing stuff can be addressed, do you agree?

Yes. Two things I don't like:

1. How I solved the psalm tests in places
2. The setParent method is not immutable. If you make it immutable - it gets lost in all cloning or starts referencing another object

[terabytesoftw]

I'm thinking of merging this PR into a develop branch, so that the missing stuff can be addressed, do you agree?

Yes. Two things I don't like:

1. How I solved the psalm tests in places
2. The setParent method is not immutable. If you make it immutable - it gets lost in all cloning or starts referencing another object

No problem, i'll solve it in the develop branch.