Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12113 closed New Feature (fixed)

Add a path name for codesnippet plugin

Reported by: Marek Lewandowski Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.4.5
Component: General Version: 4.4.0
Keywords: Cc:

Description

We should add a path name for codesnippet like "Code snippet" or simply "Snippet" (ofc it should be localized). This can be done by adding pathName property to widget definition. Sample usage is available in placeholder plugin.

Change History (14)

comment:1 Changed 4 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 4 years ago by Piotrek Koszuliński

Status: confirmedpending

Currently "pre" is displayed. What's wrong with this? It's short and I think that for most user (that will use code snippets) it's meaningful.

comment:3 Changed 4 years ago by Marek Lewandowski

I'd say simply:

  • It makes more sense for casual users, who opens editor with preserved content.
  • Lets remember that it's actually pre code rather than pre
  • It would make plugin more consistent with other CKSource provided widget plugins.

comment:4 Changed 4 years ago by Piotrek Koszuliński

Resolution: invalid
Status: pendingclosed

I still think that "pre" is ok, the same way as "p" or "h1". Elements path isn't a tool for users totally unfamiliar with HTML at the moment. If we renamed "pre" to "code snippet" we would need to change also other names.

comment:5 Changed 4 years ago by Jakub Ś

But pre is name of HTML tag so if real path is pre code then I have to agree with @m.lewandowski this can be misleading.

I think that codesnippet is much better here. We do similar for page break. We don't call it div but pagebreak in elements path.

@Reinmar please reconsider it one more time.

comment:6 Changed 4 years ago by Piotrek Koszuliński

Resolution: invalid
Status: closedreopened

Hm... It makes sense what you wrote. Indeed we have here two elements and in fact the inner is more important than the outer. Let it be "codesnippet".

comment:7 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.5
Status: reopenedconfirmed

comment:8 Changed 4 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:9 Changed 4 years ago by Artur Delura

And if we got content as follow and codesnippet plugin enabled:

<pre>Hello</pre>
<pre><code>function isEmpty() {}</code></pre>

Path for both are same: body pre, but that are two diffrent things :)

comment:10 Changed 4 years ago by Artur Delura

Status: assignedreview

Changes in branch:t/12113.

comment:11 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

I don't like the fact that the name is translatable, but I see that in image2 it is as well, so it must be in this case too.

Fixed on master with git:558f516.

comment:12 Changed 4 years ago by Anna Tomanek

Resolution: fixed
Status: closedreopened

comment:13 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reopenedclosed

I changed the element's name to 'code snippet' in git:0f6bed50bdb.

comment:14 Changed 4 years ago by Anna Tomanek

For future reference: we cannot add two-word compounds like "codesnippet" without space as translatable content -- translators will not know what to do with it and we will end up with a mixture of unreadable stuff like "fragmentkodu" and untranslated entries.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy