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

[Request] Place login and desktop in separate modules #6

Open
akvelon-georgii-ivanov opened this Issue Jun 5, 2018 · 12 comments

Comments

Projects
None yet
2 participants
@akvelon-georgii-ivanov
Copy link

akvelon-georgii-ivanov commented Jun 5, 2018

I'd like to suggest some feature request because I need to customize desktop and login views without modifying client (core) sources.

I see you use hyperapp at https://github.com/os-js/osjs-client/blob/master/src/login.js#L1
my guess it's not a good approach because core files should be separated from applications.

As well as login the desktop solution could be provided as a separate application.

@andersevenrud Hope you can implement the necessary feature or provide motivated rejection.
I could also spend some time to make this changes for you if you don't have a time for it.

@akvelon-georgii-ivanov akvelon-georgii-ivanov changed the title [Request] Separate login and desktop to dedicated application [Request] Separate login and desktop to dedicated application(s) Jun 5, 2018

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 5, 2018

Well, since these systems are provided with a Service Provider, it's fairly easy to separate these into separate modules so one can provide an option to override the default behavior. Just like the adapters work for ex. authentication and settings storage.

I'm quite busy at the moment (with other projects, both private and work) so if you want to take a look at it, by all means; go ahead 👍 I'll just keep this open and give you a shout when I start looking at it otherwise.

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 5, 2018

Btw, the login and desktop does not operate like "regular applications". They are both individual modules with their own runtime stuff (though provided via service providers).

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 5, 2018

You can customize the login as well, but maybe you wanted to do something entirely different: https://manual.os-js.org/v3/guide/login/

@akvelon-georgii-ivanov

This comment has been minimized.

Copy link
Author

akvelon-georgii-ivanov commented Jun 5, 2018

Thanks for you response, @andersevenrud !

Well, service providers could be defined as part of the core but used externally from desktop and login apps.
I mean, I'd like to use @osjs/client as a package from separated Login and Desktop apps and no need to modify @osjs/client source to change layout and functionality of login and desktop components.

Also, this feature could help to make system startup without the desktop app (or change default startup application).

I'll link pull request to this issue.

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 5, 2018

Well, service providers could be defined as part of the core but used externally from desktop and login apps.
I mean, I'd like to use @osjs/core as a package from separated Login and Desktop apps and no need to modify @osjs/core source to change layout and functionality of login and desktop components.

This is what I meant by the "adapters" in my previous comment. You'd register the service providers as per usual, but the actual service is not in @osjs/client, but their own modules.

Also, this feature could help to make system startup without the desktop app (or change default startup application).

You can already do this by removing or overriding the .register() methods in the bootstrapping.

@andersevenrud andersevenrud changed the title [Request] Separate login and desktop to dedicated application(s) [Request] Separate login and desktop to separate modules Jun 5, 2018

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 5, 2018

This is what I meant by the "adapters" in my previous comment. You'd register the service providers as per usual

You can se how this works, ex here: https://github.com/os-js/osjs-pam-auth (this is a server module, but it works the same in client).

@andersevenrud andersevenrud changed the title [Request] Separate login and desktop to separate modules [Request] Place login and desktop in separate modules Jun 5, 2018

@akvelon-georgii-ivanov

This comment has been minimized.

Copy link
Author

akvelon-georgii-ivanov commented Jun 6, 2018

I started working on it and see now that service providers use UI components that defined inside client core folder. Some of the providers (for example, Notifications) implement its own UI.

But I'd suggest another solution for osjs/client. I think that Desktop components (Login, Desktop, Notifications, etc) could be implemented separately from their providers. Providers will provide only low-level API (working with core EventHandler and communicating server).

The separate desktop application will use core EventHandler and handle events from all core registered providers.

In this way, any application could use low-level API that provided by service providers.

For example, any user app could implement login or desktop UI by using core provider functions.
That approach inherited from real operating systems and will also bring easy customization for desktop components (just by replacing desktop app). In this case, we don't need to change the source code of core or bootstrap to apply new changes.

I think that could take significant time for implementation.
What do you think about this approach?

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 6, 2018

I started working on it and see now that service providers use UI components that defined inside client core folder. Some of the providers (for example, Notifications) implement its own UI.

Yes, only the login uses Hyperapp in the @osjs/client package at the moment.

I think that Desktop components (Login, Desktop, Notifications, etc) could be implemented separately from their providers.
For example, any user app could implement login or desktop UI by using core provider functions.

Yes, this is what we've discussed above and what I demonstrated with ex the adapters. This makes it possible to pick-and-chose what functionality the providers will load.

The separate desktop application

Why do you write "desktop application" and embolden it ? I'm not sure if there are confusion about the terms here, or that you want it to behave like an application instead of just a service.

What do you think about this approach?

There are areas of the client module that could benefit from this, yes. And down the line I will probably split out the VFS stuff as well.

@akvelon-georgii-ivanov

This comment has been minimized.

Copy link
Author

akvelon-georgii-ivanov commented Jun 6, 2018

I'll also make client core.js to contain only common operations and move DOM declarations, event listeners, and other DOM operations to CoreServiceProvider to get core working without DOM directly on Node.JS.

I think separate osjs/client to 3 repos:

  1. osjs/client
  2. osjs/osjs-default-providers
  3. osjs/osjs-desktop-application
@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 8, 2018

Just an FYI, I've moved some of the code from the Core provider down to the Desktop class.

You might wanna rebase your changes :)

@akvelon-georgii-ivanov

This comment has been minimized.

Copy link
Author

akvelon-georgii-ivanov commented Jun 13, 2018

Yes, that's good.
In progress. Working on the Desktop application using hyperapp.
I'll share my progress at the end of this week.

@andersevenrud

This comment has been minimized.

Copy link
Member

andersevenrud commented Jun 15, 2018

Cool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment