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

How to skip requiring mpdf 2 #572

Closed
uldisn opened this issue Sep 17, 2016 · 13 comments
Closed

How to skip requiring mpdf 2 #572

uldisn opened this issue Sep 17, 2016 · 13 comments

Comments

@uldisn
Copy link
Contributor

uldisn commented Sep 17, 2016

Some time ago you this problem solved, but currently it back!

@kartik-v
Copy link
Owner

kartik-v commented Sep 17, 2016

Follow the instruction as mentioned in the message you would see as part of the exception. If you do not need export or PDF functionality of the grid view extension, then:

  • Either set GridView::export to false (if you are not using the entire GridView export functionality), for example:
use kartik\grid\GridView;
echo GridView::widget([
    'dataProvider' => $dp,
    'columns' => $cols,
    'export' => false
]);

OR

  • Do not include the GridView::PDF key setting inexportConfig configuration (if you are partially using export functionality without PDF support) - for example:
use kartik\grid\GridView;
echo GridView::widget([
    'dataProvider' => $dp,
    'columns' => $cols,
    'exportConfig' => [
        GridView::HTML => [],
        GridView::CSV => [],
        // do not include PDF
    ]
]);

@uldisn
Copy link
Contributor Author

uldisn commented Sep 17, 2016

Thanks!
It`s work.

@schmunk42
Copy link

The secondary solution

 'export' => [
        GridView::HTML => [],
        GridView::CSV => [],
        // do not include PDF
    ]

does not work for me, I still get the Exception.

What works is:

exportConfig' => [
                GridView::HTML => [],
                GridView::CSV => [],
                // do not include PDF
            ],

@kartik-v
Copy link
Owner

Yes that is correct.

@schmunk42
Copy link

It's good that there's a way to disable PDF export, but what I am still saying, also in regard of the other issue, is that everyone who is using kartik\grid\GridView without a hand-made config or special composer requirements is getting an exception.

Just my 2 cents.

@kartik-v
Copy link
Owner

kartik-v commented Sep 19, 2016

Could you mention what is the expectation here and what is to be improved?

The exception will be received by anyone who does not have yii2-mpdf installed in his/her app but is still trying to use PDF export functionality of yii2-grid.

@schmunk42
Copy link

The exception will be received by anyone who does not have yii2-mpdf installed in his/her app but is still trying to use PDF export functionality of yii2-grid.

Which is the default, if you do not "unconfigure" the PDF export.

@acorncom
Copy link

@kartik-v I believe @schmunk42 is suggesting that pdf export be turned off by default with a note to people who are interested in enabling it that they will need to do so. That way all other users by default don't run into this issue. It would be a breaking change though ...

@kartik-v
Copy link
Owner

kartik-v commented Sep 27, 2016

Will need to evaluate this based on feedback from the majority of the community. Currently the way it is designed, most extended functionalities of the extension are enabled by default until it is disabled by each developer in their own grid configuration.

The only difference being PDF dependency to mPDF library was earlier included in composer.json earlier to ensure extension works with PDF enabled by default. But because of the feedback in the other issue, that mPDF may not be needed by default - the mPDF package dependency was eliminated from the composer.json with the option of user installing it on his own if needed OR disable PDF.

To ensure that developers do not face an issue, there is an exception validation message included for the above change. This validates if PDF functionality is being accessed without mPDF library and I suppose its more a documentation pre-requisite now for devs when using the extension, to disable it if not needed using the options above.

Will await for more feedback and then take a call on extending or updating this.

@SamMousa
Copy link
Contributor

SamMousa commented Dec 2, 2016

How about this solution @kartik-v

  1. Add the mpdf to the recommends of grid.
  2. By default enable pdf export if mpdf is available and disable it otherwise.
  3. Make sure explicit configuration always overrides the default (so if I explicitly enable pdf export I should get exceptions).

@kartik-v
Copy link
Owner

kartik-v commented Dec 2, 2016

@SamMousa good suggestions...

will think on these.

@SamMousa
Copy link
Contributor

SamMousa commented Dec 9, 2016

@kartik-v I can create a PR if you want this improvement; it will not break backwards compatiblity in any working setup. (It will break BC for people that currently get exceptions for missing dependencies).

@kartik-v
Copy link
Owner

kartik-v commented Dec 9, 2016

Sure you can submit a PR.

SamMousa added a commit to SamMousa/yii2-grid that referenced this issue Dec 12, 2016
kartik-v added a commit that referenced this issue Dec 14, 2016
Fixed issue #572: Silently disable PDF when dependency is not available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants