-
Notifications
You must be signed in to change notification settings - Fork 444
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
Add tinymce editors to article title fields #3544
Conversation
rows="{$FBV_rows|escape}" | ||
cols="{$FBV_cols|escape}" | ||
class="localizable {$FBV_class} {$FBV_height}{if $FBV_validation} {$FBV_validation|escape}{/if}{if $formLocale != $currentLocale} locale_{$formLocale|escape}{/if}{if $FBV_rich && !$FBV_disabled} richContent{if $FBV_rich==="extended"} extendedRichContent{/if}{/if}" | ||
class="localizable {$FBV_class} {$FBV_height}{if $FBV_validation} {$FBV_validation|escape}{/if}{if $formLocale != $currentLocale} locale_{$formLocale|escape}{/if}{if $FBV_rich && !$FBV_disabled} richContent{if $FBV_rich==="extended"} extendedRichContent{/if}{if $FBV_rich==="oneline"} onelineRichContent{/if}{/if}" | ||
{if $FBV_disabled} disabled="disabled"{/if} | ||
{if $FBV_disabled} disabled="disabled"{/if} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are two identical lines here now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
must be something that happened when I committed the code, I will send a fix for this later today...
@@ -247,6 +246,7 @@ | |||
// html source, for example (both actions open a new window). | |||
if ($(tinyMCEObject.getContainer()).find('iframe').attr('id') == | |||
$(document.activeElement).attr('id')) { | |||
$(document.activeElement).find('div.mce-tinymce-inline').hide(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on in this code? I can see you're hiding the inline editor, but as far as I can tell, that's when the field is still the active element. I must be misunderstanding something here so just looking for some clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments here: #2564 (comment)
In short that is an attempt to solve the problem with the multilingual forms. Have to say that I did not check yet whether it breaks the single locale title field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I saw that. I'm just still confused. Doesn't the conditional it's wrapped in mean that it only fires when it's still the active element? Can you describe what's happening here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I look at it, I have no idea 🤣
But I removed that and added this: https://github.com/pkp/pkp-lib/pull/3544/files#diff-83df7d7f7c760e14fa91323920849101R235
The problem was that the editor appeared/disappeared as expected, however clicking some button (bold) closed the multilingual popover. So I made the closing js there conditional.
Worked for me, can you check that out?
This looks good @ajnyga! I just took it for a spin in IE10/11, Chrome and Firefox and it works pretty well, bar some hiccups that are mostly related to how we handle multi-lingual inputs in general. In IE 10/11, when tabbing from the Prefix field into the Title field, the inline tools are misplaced, because the fields jump up-and-down with the opening/closing of multilingual inputs: Also, I noticed that tabbing out of the control doesn't close the window. But this seems to be the case for any TinyMCE multingual field with the current I don't see either of these things as blockers. Do you remember the issue # where we discussed this? I know we had some indexing issues to think about as well. |
@NateWr Check the new version when you have the time. I can then proceed to editing the form validators (not sure yet if using div as an input field will be a problem) and after that we could implement the changes mentioned on the forum that enable the correct output of italics in titles in different places. The last thing is to take care of the different export plugins. |
I just did a quick test and it looks good. 👍
Yeah I noticed that and I guess it's unfortunate but not a deal-breaker. We could get around it by resetting the position whenever a scroll occurs but it seems like a lot of fragile code for little benefit. |
Great, I will check the form validators next. |
@NateWr, tabbing through the fields should work now. Textareas do not seem to close that way though as you noticed. |
See #3265 for old pr.
work in progress...