Skip to content
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

Widgets #52

Merged
merged 10 commits into from Sep 4, 2019

Conversation

@terabytesoftw
Copy link
Contributor

commented Aug 29, 2019

Q A
Is bugfix? ✔️
New feature?
Breaks BC? ✔️
Tests pass? ✔️
@terabytesoftw

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

Syntax Proposal:

<?php NavBar::begin()
    ->brandLabel($params['app.name'])
    ->brandUrl($params['app.home.url'])
    ->options([
        'class' => 'navbar navbar-dark bg-dark navbar-expand-lg',
    ])
    ->init();

    echo Nav::widget()
        ->items($menuItems)
        ->options([
            'class' => 'navbar-nav float-right ml-auto'
        ]);

NavBar::end(); ?>

Widget::begin() need init() --> Widget::begin()->init()/Widget::end();
Widget::widget() need return in run(): string.
@samdark
samdark approved these changes Sep 1, 2019
Copy link
Member

left a comment

Looks good so far 👍

src/View/DynamicContentAwareTrait.php Outdated Show resolved Hide resolved
src/Widget/Block.php Outdated Show resolved Hide resolved
src/Widget/Widget.php Outdated Show resolved Hide resolved
src/Widget/Widget.php Show resolved Hide resolved
src/Widget/Widget.php Outdated Show resolved Hide resolved
src/Widget/Widget.php Outdated Show resolved Hide resolved
@@ -70,10 +77,12 @@ protected function setUp(): void
$this->aliases = $this->container->get(Aliases::class);
$this->assetManager = $this->container->get(AssetManager::class);
$this->eventDispatcher = $this->container->get(EventDispatcherInterface::class);
$this->listenerProvider = $this->container->get(ListenerProviderInterface::class);

This comment has been minimized.

Copy link
@samdark

samdark Sep 3, 2019

Member

Seems listenerProvider isn't declared as class property.

This comment has been minimized.

Copy link
@samdark

samdark Sep 4, 2019

Member

I mean there's no private $listenerProvider in the class itself.

This comment has been minimized.

Copy link
@terabytesoftw

terabytesoftw Sep 4, 2019

Author Contributor

Yes, but I need it to test the events.

tests/bootstrap.php Outdated Show resolved Hide resolved
@terabytesoftw

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

The widgets that are missing are those that have to do with activerecord and cache they are:

  • ActiveField.php.
  • ActiveForm.php.
  • ActiveFormClientScript.php.
  • FragmentCache.php.
  • InputWidget.php.
  • LinkPager.php.
  • LinkSorter.php.
@samdark

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

  1. Let's do active forms separately. These require great deal of rethinking because Yii 3 isn't limited to Active Record.
  2. FragmentCache could be done right now (or separately if you prefer). We have everything for it.
  3. Pager and sorter should be done later when https://github.com/yiisoft/data is ready and will be part of https://github.com/yiisoft/yii-dataview.
@samdark samdark changed the title Draft widgets no merge. Widgets Sep 4, 2019
@samdark samdark marked this pull request as ready for review Sep 4, 2019
@samdark samdark merged commit 9e00c63 into yiisoft:master Sep 4, 2019
1 of 4 checks passed
1 of 4 checks passed
Travis CI - Pull Request Build Errored
Details
Scrutinizer Running
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/styleci/pr The analysis has passed
Details
@terabytesoftw terabytesoftw deleted the terabytesoftw:Terawidget branch Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.