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

[Panel] Changing a date in Edge does not bring up save bar #1712

Closed
fabianmichael opened this issue Apr 23, 2019 · 26 comments

Comments

@fabianmichael
Copy link
Contributor

commented Apr 23, 2019

Describe the bug
When changing the value of a date field using Microsoft Edge, the save bar is not displayed.

To Reproduce
Steps to reproduce the behavior:

  1. Install the starterkit and open the edit view for any page with a date field.
  2. Change the day/year etc. in the date field.
  3. No save bar appears. However, it is displayed, once you edit any other field of the page.

Expected behavior
The save bar should be displayed, as with any other changed value.

Kirby Version
3.1.3

Console output
none

Desktop (please complete the following information):

  • OS: Windows 10 (Windows Update says, it’s the latest version)
  • Browser: Microsoft EdgeHTML 17.17134 (This number also appears above the version without any further description what it means: 42.17134.1.0)

@distantnative distantnative modified the milestone: 3.1.4 May 3, 2019

@distantnative distantnative added this to the 3.2.3 milestone Jun 2, 2019

@bastianallgeier bastianallgeier modified the milestones: 3.2.3, 3.2.4 Jul 10, 2019

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

@lukasbestle @bastianallgeier am I right that this issue has not been tackled cause no one of us has a windows pc with edge at hands to test and debug? How dill we deal with that situation?

@lukasbestle

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

I haven't tackled it since I'm not working on the Panel. ;)
I'd have a Windows 10 VM though (https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/).

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

I tried to find the source of the problem and found the issue:

I noticed it didn't work even though I put a breakpoint to @input event.
So not triggerring @input event in select with date field type.
But works with selecting date from calendar.
Then i confirmed with adding standalone select field type and didnt work too as you can see in screencast.

edge

Console: No any errors.

So i searched in google and i found out why it doesn't work:

https://stackoverflow.com/a/54516876/1769465

Versions

  • Kirby 3.2.3-RC.1
  • Windows 10
  • Microsoft Edge 44.17763.1.0
  • Microsoft EdgeHTML 18.17763
@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

You are a hero, thanks for tracking this down!

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

You're wellcome 😅

After this stage there is something you want me to test, I can gladly do.

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

@afbora and @lukasbestle:
Could one of you test it with these dist files. Just replace the panel/dist/ directory and make sure to remove the media/panel/ folder. I hope this small change will fix it:

dist.zip

@distantnative distantnative self-assigned this Aug 6, 2019

distantnative added a commit that referenced this issue Aug 6, 2019
@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

I have tested now and nothing happened for me on fresh install, still same :((

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

Hmm... I followed exactly vuejs/vue#7248 – not sure why this would not work then.

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

Has it worked successfully on a platform like jsbin/jsfiddle?

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

I wouldn't know how to test the Panel over there.

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

https://jsbin.com/zicevezani/edit?html,js,output

I guess this sample works with Edge browser.
When I click it prints the selected one.

edge

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

That's how we have had it so far in Kirby – so no idea why this is working for you in the example but not in the Panel.

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

As you said, the event seems to be working on the dropdown element.

Can we create a copy of Kirby select field element on jsbin? I tried, but I failed.
If it is not difficult, maybe we can find the error more easily when creating this copy.

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

@afbora I created one with the new implementation, could you give it a try: https://jsbin.com/zicumixara/edit?html,js,output

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

@distantnative Great! I have tested now. Chrome working with output. But no output (empty) in Edge with following console error:

Expected identifier, string or number (line 51)

Untitled-1

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

Hmm, ok this really won't work debugging just going back and forth since I have no clue so far where even that error is coming from. I am afraid I won't be able to continue working on this before getting my hands on Edge myself (in the macOS Edge beta it works fine).

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

You're right, it's hard to find a solution like this.

By the way, worked with output and change event when I tried to remove the ...this.$listeners line.

Untitled-2

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

One more try, @afbora: Does this work: https://jsfiddle.net/c98yt5de/5/?

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Same with jsbin. No output and Expected identifier, string or number error that ...this.$listeners line.

When i remove that line, html and console output working with not errors 👍

edge

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Actually, I have been so much focussing on the @input event that was mentioned in those links as not supported by Edge. But it seems more that the problem is the spread operator:

Screen Shot 2019-08-09 at 15 10 37

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

I replaced the spread syntax with Object.assign:
https://jsfiddle.net/yj5od7tb/

I don't want to remove that line, it might break something else.

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

I don't want to remove that line, it might break something else.

Just to show you how it works.

Now perfectly works, tested with Edge, Chrome, Firefox 👍 👍 👍

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Hopefully (!) one last check:
Does it actually work now in the Panel as well with these dist/ files?
dist.zip

@afbora

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Working perfect with panel and no console error 👍

@distantnative

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

🎉 thanks a lot!

distantnative added a commit that referenced this issue Aug 9, 2019
distantnative added a commit that referenced this issue Aug 9, 2019
bastianallgeier added a commit that referenced this issue Aug 9, 2019
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.