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

Livedev multibrowser #4

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Livedev multibrowser #4

wants to merge 18 commits into from

Conversation

sebaslv
Copy link
Owner

@sebaslv sebaslv commented Nov 18, 2014

This is an alternative approach to #3 for including livedev2 in Brackets core as an experimental implementation as it was discussed in njx/brackets-livedev2#24.

  • keeps current LiveDevelopment untouched -> dependent extensions do not have to be modified
  • includes the entire livedev2 implementation under /LiveDevelopment/MultiBrowserImpl folder which is probably not the more fancy way (under a second level) but allows integration be less intrusive.
  • move livedev2/LiveDevelopment.js out to LiveDevelopment root folder and renamed it to LiveDevMultiBrowser.js
  • deleted livedev2/main.js
  • add quick modification to main.js to allow switching implementations:
    • there is a new config key added which allows enabling multibrowser impl (we can change that to be a preference if needed)
    • (not implemented yet) need to align some states to make the UI status update work

@busykai, please review it. Thanks!

It now allows switching btw implementations by setting a preference.
livedev.multibrowser set to true enables MultiBrowserImpl.
@sebaslv
Copy link
Owner Author

sebaslv commented Nov 19, 2014

Preference now added livedev.multibrowser set to true will enable the experimental implementation. Value change is being handled (will restart the session with the new selected implementation) but if the current open document is preferences file it will ask for an HTML file which is expected.

Remove logic for launching browser from transport. It now relies on
NativeApp.openLiveBrowser
@sebaslv
Copy link
Owner Author

sebaslv commented Nov 19, 2014

Removed logic for launching default browser from NodeSocketTransportDomain since we're now in Brackets core we can rely on existing NativeApp.openLiveBrowser. I think we should include launch of multiple browsers at this point making it an optional choice when enabling this implementation.

node.remove() is not supported on IE, using alternative
node.parentNode.removeChild(node) in that case
@sebaslv
Copy link
Owner Author

sebaslv commented Nov 19, 2014

Add fix for make it work on IE as per it was analyzed in njx/brackets-livedev2#20.
It should probably be a separated PR once livedev2 get merged into core. I'm including it in this initial review so we keep it in the radar of the integration.

Add integration tests for initial connection and related documents.
- Add close window based on NativeApp.closeLiveBrowser for mac
- Move launch from protocol to LiveDevelopment based since already
relies on NativeApp.openLiveBrowser
- move reloadCSS implementation from ExtendedRemoteFunctions to
  LiveDevProtocolremote with the rest of the remote commands
- add method setStylesheetText to protocol to make it more
  representative of the implementation and also align it with CDT name
- align LiveCSSDocument to the new protocol method
- nodes created with new stylesheet text are now removed when the
  stylesheet is removed
- make updates on import-ed CSS work in Firefox
It also implements getStylesheetText on remote protocol and fixes
respond (there was no previous usage of it).
test for pushing in-memory changes
Conflicts:
	src/LiveDevelopment/main.js
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.

1 participant