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

Inconsistent behavior for panel title #13

Closed
larsmagu opened this issue May 4, 2022 · 12 comments
Closed

Inconsistent behavior for panel title #13

larsmagu opened this issue May 4, 2022 · 12 comments

Comments

@larsmagu
Copy link

larsmagu commented May 4, 2022

There seems to be some strange behavior with regards to setting the titles of the different panel tabs.

  • If you first add the titles of the Properties first, then add the Alternative text, the panel title won't update according to the alternative text. Instead the title is set to be the Property title. The user needs to save the content in order for it to update.
  • If you only have one Property set, the panel title won't update until the user saves the content or switches tabs in the browser. If the user adds more than one Property, the panel title is set once the 2nd Property is set for the panel.
@otacke
Copy link
Owner

otacke commented May 4, 2022

@larsmagu I'll look into that. If I recall correctly, I had to jump through some hoops for getting the behavior I wanted (cmp. h5p/h5p-editor-php-library#113 and h5p/h5p-editor-php-library#134) and maybe there's still a bug lurking in the dark.

@otacke
Copy link
Owner

otacke commented May 4, 2022

@larsmagu Could you please share what you changed in the H5P.com fork of the editor core (not even asking why there's a fork instead of just one version ;-) )? I don't have access to that change, but knowing what you did to fix the symptom might help to find the cause.

@larsmagu
Copy link
Author

larsmagu commented May 4, 2022

This has been handled by Ravi. The fix was done to h5pcom-editor-php-library, but will probably be reverted. The fault of this issue seems to be related to DOMupdate and focus. I believe the error reported by me exists regardless of this change.

@otacke
Copy link
Owner

otacke commented May 4, 2022

@larsmagu Just recorded this a minute ago: https://drive.google.com/file/d/1oIg0CxtuEMVqIjG1qLlbUOaZwhG8Li8L/view?usp=sharing Please just let me know if that's what you are referring to.

The tabs of the Vertical Tab widget will not update live as you type, but when a change event is fired (e.g. tabbing out of the text field, switching to some other tab), and they will be created from the parameters when you save/load content, of course. That's normal H5P behavior, yes. And then there's h5p/h5p-editor-php-library#134 which you may find odd, too.

Please just let me know if there's anything that I should do, or close the issue here otherwise.

@larsmagu
Copy link
Author

larsmagu commented May 5, 2022

Hi Oliver,

I've tried to reproduce the issue. Please see the attached video for reference. The video showcases both scenarios reported.

  • The first one is illustrated when I create a new panel tab. I update all fields, but the panel label is not set. This happen sometimes, and sometimes not. The behavior seems to be inconsistent. I'm not sure if this depend on where I click when I update the properties of the panel?
  • The second one is illustrated when I create a new panel and update one of the property fields first. The panel label is set to equal the first property set. Once I then add Alternative text, the panel label is not updated. Even if I click inside one of the property fields, it is not updated. Instead I have to save for the panel labels to update.

Recording

@otacke
Copy link
Owner

otacke commented May 5, 2022

@larsmagu Thanks! I'll try to reproduce this. I had included a function to work around h5p/h5p-editor-php-library#134 so it doesn't necessarily choose the first text field's value for the panel but the 'best one': image metadata title > image alt text > text entry. That function is most likely involved. Might be something related to the not yet public changes in the H5P.com exclusive editor core (related to HFP-3508), so if I cannot reproduce this, someone on your end may either need to provide me with the changes or you'll take a look yourself. Guess you'd first check if one of the listeners in https://github.com/otacke/h5p-editor-info-wall/blob/master/src/scripts/h5peditor-info-wall.js#L100 is not firing when it is supposed to (then that listener needs correcting) or https://github.com/otacke/h5p-editor-info-wall/blob/master/src/scripts/h5peditor-info-wall.js#L148 is not determining the title as it should or maybe even some conflict between https://github.com/otacke/h5p-editor-info-wall/blob/master/src/scripts/h5peditor-info-wall.js#L140 and some other code that changes the field.

@larsmagu
Copy link
Author

larsmagu commented May 5, 2022

@otacke Let me know whether you're able to reproduce or not, and I'll assist accordingly.

@otacke
Copy link
Owner

otacke commented May 5, 2022

@larsmagu Just looked into it.

The issue seems to be the way I tried to keep track of changes to the CKEditor fields of the H5P editor core widget. That fields get destroyed and replaced by new ones from time to time, so the listener that I attach to the original versions can't detect anything anymore. Thus the panel title is not updated anymore - and the change that Pål suggested to fix the other issue should fix this as well. Am about to confirm that right now.

@otacke
Copy link
Owner

otacke commented May 5, 2022

@larsmagu Yup, Pål's solution for the other issue prevents deleting and re-adding fields, so this should fix both problems with one strike. Would be cool if you could confirm that though. You'd need to get the latest master versions of https://github.com/otacke/h5p-editor-info-wall and https://github.com/otacke/h5p-editor-info-wall-passive-list

@larsmagu
Copy link
Author

larsmagu commented May 5, 2022

Perfect! Nothing's better than a 2-for-1

@otacke
Copy link
Owner

otacke commented May 5, 2022

Who knows, maybe it fixed a bug that nobody has detected so far and it's a 3-for-1 ;-)

@otacke
Copy link
Owner

otacke commented Jun 14, 2022

@larsmagu I guess I can close this one. If not, please holler!

@otacke otacke closed this as completed Jun 14, 2022
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

2 participants