Skip to content

Conversation

bjarnef
Copy link
Contributor

@bjarnef bjarnef commented Oct 17, 2023

Description

Added missing icons shown at toolbar in configuration of richtext editor.

image

The tinymce doesn't contain a "default" property, but IconManager directly. Not sure why, but when when output to console in render function, it seems to log twice.

Before

image

After

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Motivation and context

How to test?

Screenshots (if appropriate)

Checklist

  • If my change requires a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@bjarnef
Copy link
Contributor Author

bjarnef commented Oct 17, 2023

@iOvergaard @loivsen seeems like an oversight?

@nielslyngsoe
Copy link
Member

@bjarnef looks good to me, there is though a TypeScript issue. I assume TinyMCE is not TypeScript, if that's true then I recommend casting you import to any. otherwise, try to see what you can do to get the right type.

@bjarnef
Copy link
Contributor Author

bjarnef commented Oct 17, 2023

@nielslyngsoe ahh, I see the failed test.

I noticed elsewhere in code where it use type instead.

import type { tinymce } from '@umbraco-cms/backoffice/external/tinymce';

image

but then I get an error:

'tinymce' cannot be used as a value because it was imported using 'import type'.ts

Copy link
Member

@nielslyngsoe nielslyngsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks!

auto-merge was automatically disabled October 26, 2023 12:22

Head branch was pushed to by a user without write access

…property-editor-ui-tiny-mce-toolbar-configuration.element.ts

Co-authored-by: Niels Lyngsø <niels.lyngso@gmail.com>
@bjarnef
Copy link
Contributor Author

bjarnef commented Oct 26, 2023

@nielslyngsoe is there a reason elsewhere in code it use type?

import type { tinymce } from '@umbraco-cms/backoffice/external/tinymce';

@nielslyngsoe
Copy link
Member

@bjarnef when we apply type to the import statement it basically means that we do not want a real import, we just want to borrow the type in the TypeScript compile check. :-) So if the code is actually used, then we should not append type to the import statement :-)

Copy link
Collaborator

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iOvergaard iOvergaard enabled auto-merge November 8, 2023 10:17
…same as we do on the production build (this helps with tinymce)
@iOvergaard iOvergaard merged commit 4694565 into umbraco:main Nov 8, 2023
@bjarnef bjarnef deleted the bug/tinymce-configuration-icons branch November 8, 2023 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants