contributte / menu-control

🍔 Menu and breadcrumb components for Nette framework (@nette)
MIT License
28 stars 20 forks source link

Contributte(s) #41

Closed f3l1x closed 3 years ago

f3l1x commented 4 years ago
f3l1x commented 4 years ago

Because contributte is all about Nette stuff I would prefer to rename this pkg to contributte/menu-control or menu-component.

It would be much better name and suits to our plan about some UI components ecosystem.

WDYT @foxycode ?

foxycode commented 4 years ago

@f3l1x What about just menu? Datagrid doesn't have control in name too :)

f3l1x commented 4 years ago

Well, I hope there will be more components and we're running out of names. For example calendar, contributte/calendar is so common name, don't you think?

foxycode commented 4 years ago

@f3l1x Does it make sense to have more components for menu in one org?

f3l1x commented 4 years ago

Hi @foxycode. Nope, no more menu components are planned. But take a look at calendar component. From the name you don't know if it's GUI control or the business logic handling dates and some calculations.

Thus, I'd prefer to have -control postfix.

But, this is my point of view. If you're not into it I'd understand a we can rename it to contributte/menu.

Let me know please. We're about to finish this transformation.

foxycode commented 4 years ago

@f3l1x Ok, let it be menu-control :)

f3l1x commented 4 years ago

Well done. Renamed. Let me know, I will publish it to packagist as soon as possible.

foxycode commented 4 years ago

@f3l1x You can publish on packagist.

f3l1x commented 4 years ago

Done https://packagist.org/packages/contributte/menu-control

f3l1x commented 4 years ago

ping @foxycode, could you please work on phpstan?

foxycode commented 4 years ago

@f3l1x There are more important things, but yes, I'm planning it. I currently have daughter on vaccation here. Will have more time from half of August.

foxycode commented 4 years ago

@f3l1x I tried to add PHPStan and fix errors, but there are lot of things I don't like. Are you really making them less readable in your code just to shut up PHPStan?

Example:

------ ----------------------------------------------------------------------------------------------- 
  Line   MenuItem.php                                                                                   
 ------ ----------------------------------------------------------------------------------------------- 
  83     Only booleans are allowed in &&, Nette\Application\UI\Presenter|null given on the right side.
  83     Only booleans are allowed in &&, string|null given on the left side.
  89     Only booleans are allowed in an if condition, array<string> given.
 ------ -----------------------------------------------------------------------------------------------

Current code:

// $this->getAction() returns ?string
// $this->menu->getActivePresenter() returns ?Presenter
if ($this->getAction() && $this->menu->getActivePresenter()) {
}

Fixed for PHPStan:

if ($this->getAction() !== null && $this->menu->getActivePresenter() !== null) {
}

I don't like making this longer because error is not possible in this case. What do you think?

foxycode commented 3 years ago

@f3l1x Any ideas?

f3l1x commented 3 years ago

I will take a look, could you @foxycode open PR? We can set to ignore some rules/complains.

foxycode commented 3 years ago

@f3l1x Not sure where I should open PR, problem is in phpstan-strict-rules package. I added soem exceptions and fixed the rest of the errors, PHPStan is now added to QA workflow.