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

Update browsersync-update-content.js #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benwest
Copy link

@benwest benwest commented Feb 11, 2021

The method of detecting the panel didn't work for me, maybe Kirby 3.5 broke it? This works.

The method of detecting the panel didn't work for me, maybe Kirby 3.5 broke it? This works.
@antoine1000
Copy link
Member

antoine1000 commented Feb 20, 2021

Hello @benwest,

Thanks for your interest in Kirby Webpack. Can you be more specific when you say "method of detecting the panel didn't work"? What didn't work exactly? The panel seems to work correctly on my side when I connect to localhost:8080/panel (with Kirby 3.5.3)

@benwest
Copy link
Author

benwest commented Feb 22, 2021

The panel would reload whenever the .lock file was written, because inPanel() === false.

@benwest
Copy link
Author

benwest commented Feb 22, 2021

I just did a fresh install of Kirby 3.5.1, went to /panel, pasted inPanel's definition into the console, and called it - false.

@arnaudjuracek
Copy link
Member

arnaudjuracek commented Feb 22, 2021

The panel would reload whenever the .lock file was written, because inPanel() === false.

I got the same issue and your PR fixes it.

The only issue I see with your fix is if someone have a panel key in their global namespace, in which case the inPanel() function will cause a false positive.

@benwest
Copy link
Author

benwest commented Feb 23, 2021

True, neither method is ideal, but these are edge cases and I don’t think this way is any worse.

Speculatively what would be the clean/correct way to do this? A Kirby plugin?

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

Successfully merging this pull request may close these issues.

3 participants