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

Update Editor Custom Toolbar Documentation #8905

Closed
abhaykeerthy12 opened this issue May 31, 2020 · 6 comments · Fixed by #9867
Closed

Update Editor Custom Toolbar Documentation #8905

abhaykeerthy12 opened this issue May 31, 2020 · 6 comments · Fixed by #9867
Assignees
Labels
LTS-FIXED-10.0.9 Fixed in PrimeNG LTS 10.0.9 Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@abhaykeerthy12
Copy link

Current behavior
Current editor / quilljs example in documentation for custom toolbar not working. the classes are old. but primereact has updated quill documentaion

Expected behavior
Documentation should provide new editor custom toolbar example

What is the motivation / use case for changing the behavior?
Documentation is old for editor

  • Angular version:
    9.1.9

  • PrimeNG version:
    9.1.0

@yigitfindikli yigitfindikli added the Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add label Nov 23, 2020
@MikeMatusz
Copy link
Contributor

I think the editor might actually be broken here. It still supports the deprecated <p-header> syntax, but based on the following template (from editor.ts), you actually need provide both a pTemplate="toolbar" (which won't be rendered) and a pTemplate="header" (which is rendered, but only if the toolbar is provided). In v10, there was no way to provide the headerTemplate so you had to continue to use <p-header>, now in v11 you need to provide both for it to work. Based on the showcase, it seems like toolbarTemplate should be removed and there should only be headerTemplate?

 <div class="p-editor-toolbar" *ngIf="toolbar || toolbarTemplate">
    <ng-content select="p-header"></ng-content>
    <ng-container *ngTemplateOutlet="headerTemplate"></ng-container>
</div>

@MikeMatusz
Copy link
Contributor

MikeMatusz commented Jan 26, 2021

@yigitfindikli Please review my comment above and respond with what type of change you would accept. Considering the showcase is currently broken, I'm pretty sure it's not working the way you think it should.

For clarity, the current situation is this:

  • If you provide a <p-header> element, it works as it always has, but this is listed as deprecated.
  • If you provide only a `pTemplate="toolbar" (which is not documented) it will not be rendered.
  • If you provide only a `PTemplate="header" (documented, used by the Showcase) it will not be rendered.
  • If you provide both a pTemplate="toolbar" and pTemplate="header" the header template will be rendered. The toolbar template will never be rendered, but is needed to trigger showing the header template.

My change would have continued to support <p-header>, and supported pTemplate="header" as documented. Do I just also need to support pTemplate="toolbar"?

Maybe something like this:

 <div class="p-editor-toolbar" *ngIf="toolbar || headerTemplate || toolbarTemplate">
    <ng-content select="p-header"></ng-content>
    <ng-container *ngTemplateOutlet="headerTemplate || toolbarTemplate"></ng-container>
</div>

@abhaykeerthy12
Copy link
Author

Sorry for the late reply, for a good developer experience, its better to drop support for pTemplate="toolbar", and document <p-header> and pTemplate="header" properly with respect to quill js documentation, like how to have custom toolbar options and so on

@MikeMatusz
Copy link
Contributor

@abhaykeerthy12 Looks like the documentation was just recently updated to use pTemplate="header" (#9795), it just isn't actually working right now due to the issue I've described. <p-header> is deprecated, noted here in the Changelog via #9096 and on the migration guide.

The rejected PR does exactly that, drops support for pTemplate="toolbar", makes pTemplate="header" actually work, and keeps support for the deprecated <p-header>.

@abhaykeerthy12
Copy link
Author

Thanks for reminding that <p-header> is deprecated. Then it will be better to have only one way by using pTemplate="header", as it would remove lot of dilemma regarding the editor usage.

@s4m0r4m4
Copy link
Contributor

s4m0r4m4 commented Feb 5, 2021

I've run into this same issue. Noticed that using somewhat works but is deprecated. I've tried messing around with the quill modules but didn't make much headway. @yigitfindikli this should be labeled as a bug, not an enhancement. There does not seem to be a supported way to customize the Quill Editor toolbar.

@yigitfindikli yigitfindikli self-assigned this Feb 19, 2021
@yigitfindikli yigitfindikli added this to the 11.3.0 milestone Feb 19, 2021
yigitfindikli added a commit that referenced this issue Feb 19, 2021
Fixed #8905 - Update Editor Custom Toolbar Documentation
@yigitfindikli yigitfindikli added LTS-FIXED-10.0.8 Fixed in PrimeNG LTS 10.0.8 LTS-FIXED-10.0.9 Fixed in PrimeNG LTS 10.0.9 and removed LTS-FIXED-10.0.8 Fixed in PrimeNG LTS 10.0.8 LTS-PORTABLE labels Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-FIXED-10.0.9 Fixed in PrimeNG LTS 10.0.9 Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants