workhorsy / comic_book_reader

A browser based touch friendly comic book reader
GNU Affero General Public License v3.0
21 stars 7 forks source link

Component-Oriented UI #107

Open btzr-io opened 7 years ago

btzr-io commented 7 years ago

Simplify UI with modular and reusable components:

Todo

btzr-io commented 7 years ago

https://github.com/workhorsy/comic_book_reader/tree/polymer is this branch still active?

workhorsy commented 7 years ago

This is a good idea. I have been looking into different libraries to do this: angular, react, vue, polymer, et cetera. Unfortunately right now I don't know enough about either to properly use them.

workhorsy commented 7 years ago

@btzr-io That polymer branch was accidentally pushed to github. I don't think there is anything even in it. It was just for a test. I'm not even sure if polymer is the right component library to use.

btzr-io commented 7 years ago

Ok, here's a basic implementation of react (not ready yet): https://github.com/btzr-io/comic_book_reader/tree/react

Run for development

http://localhost:3000/

npm start
btzr-io commented 7 years ago

Now we just need to split UI into react-components:

workhorsy commented 7 years ago

Since you started porting to React, I've been looking into it more. Surprisingly it has a problem with its license. From what I understand, it uses the 3 clause BSD license with a patent rider clause added:

https://en.wikipedia.org/wiki/React_(JavaScript_library)#Licensing

This would make it so Facebook could terminate someones right to use React, if Facebook thinks they violated one of Facebook's patents. Or if they made a patent claim against Facebook.

It looks like there is a recent effort to fix this: https://github.com/facebook/react/issues/10191

Until this is resolved, there is no way I can support using React. You can continue changing it to use React if you want. But there is no way I can merge this into master with the license problem.

btzr-io commented 7 years ago

Well, we could use: preact

Preact

Fast 3kB alternative to React, with the same ES2015 API.

And if facebook ever fix that issue we could migrate to react:

Preact-compat

This module is a compatibility layer that makes React-based modules work with Preact, without any code changes.

btzr-io commented 7 years ago

Preact License: MIT License

workhorsy commented 7 years ago

I'll look into preact. Thanks

workhorsy commented 7 years ago

Right now I'm looking at a bunch of different component libraries. Here are a few requirements that will need to be met for us to select one:

btzr-io commented 7 years ago

I think preact it's a really good option (lightweight, fast, react-compatible, works with redux) but I only can think about: react, riot, vue.js, polymer

License

Preact License: MIT License

Setup

https://github.com/developit/preact#getting-started

Preact-cli

Start building a Preact Progressive Web App in seconds

https://github.com/developit/preact-cli

Benchmark

http://developit.github.io/preact-perf

Browser support

Browsers

btzr-io commented 7 years ago

Riot.js

Framework size comparison

Riot | 3.6.1 | 10.43kb

Browser support

Riot is supported by all modern browsers and it does not require any additional polyfill

License MIT

codyhatch commented 7 years ago

This would make it so Facebook could terminate someones right to use React, if Facebook thinks they violated one of Facebook's patents. Or if they made a patent claim against Facebook.

That's incorrect. Facebook cannot terminate your right to use React, they can only terminate your right to use any patents which they may or may not have on the concepts underlying React. And that termination does not happen if Facebook thinks you've violated their patents, only if you make a patent claim against Facebook. That's clarified explicitly here.

Q: Does termination of the additional patent grant in the Facebook BSD+Patents license cause the copyright license to also terminate?

A: No.

j127 commented 7 years ago

they can only terminate your right to use any patents which they may or may not have on the concepts underlying React

I think that it isn't wise to have any of that hanging over software projects, especially as those PATENTS files get scattered throughout dependencies (Immutable, etc.). It creates a power imbalance, since Facebook could violate your patents, and then you would have to figure out what that untested file really means in practice.

Until those PATENTS files are removed, many companies won't be able to use open-source software that depends on Facebook libraries.

workhorsy commented 7 years ago

@btzr-io What do you think of polymer 2? It seems to be closest to what vanilla Web Components will be. The only down side I can see is that it still uses Bower, which is basically dead.

btzr-io commented 7 years ago

http://riotjs.com/compare/#polymer

  • It still uses Bower, which is basically dead.

  • Some features used by Polymer are not yet supported natively in all browsers.

    • The Web Components syntax is experimental and complex.

I never used polymer before, I'm looking at the docs...

workhorsy commented 7 years ago

It looks like they are comparing polymer 1.8. Polymer 2.0 is a rewrite that is relatively new and allegedly solves performance problems.

btzr-io commented 7 years ago

Polymer(v1.8.0) + WebComponents(v0.7.24) is 6x bigger than Riot

workhorsy commented 7 years ago

I'm not really concerned about size, since we will be shipping around 3MB of code just to uncompress Rar, Zip, Tar, and view PDF files.

btzr-io commented 7 years ago

Ok, I'm not sure how to implement it yet... I rather prefer to use other framework (react / preact as I suggested) but if you think this is the best option, I'll be glad to help you with the components

workhorsy commented 7 years ago

@codyhatch I'm not sure what you mean. I am not a lawyer btw. In my reading of the PATENTS file there are several ways in which the license would terminate unfairly:

  1. They define "Patent Assertion" as "including a cross-claim or counterclaim". Section (i) "against Facebook or any of its subsidiaries or corporate affiliates". Therefore any counter claim against Facebook would terminate the license.

  2. Section (ii) would terminate the license if the user ever sued Facebook for infringing their own patent. This includes any "corporate affiliates". This could be any company Facebook has any relationship with.

  3. Section (iii) says that the license would only NOT expire if the counter claim was "against that party that is unrelated to the Software". I read that as: Counter claims terminate the license if they are against the people that wrote/own the code (Facebook).

I'm sure there are more holes. I seems like this entire PATENTS file is designed to look like it is protecting Facebook from being attacked by patents claims. While in reality it is filled with double negatives, that stop people from defending themselves from attack by Facebook.

The license granted hereunder will terminate, automatically and without notice,
if you (or any of your subsidiaries, corporate affiliates or agents) initiate
directly or indirectly, or take a direct financial interest in, any Patent
Assertion: (i) against Facebook or any of its subsidiaries or corporate
affiliates, (ii) against any party if such Patent Assertion arises in whole or
in part from any software, technology, product or service of Facebook or any of
its subsidiaries or corporate affiliates, or (iii) against any party relating
to the Software. Notwithstanding the foregoing, if Facebook or any of its
subsidiaries or corporate affiliates files a lawsuit alleging patent
infringement against you in the first instance, and you respond by filing a
patent infringement counterclaim in that lawsuit against that party that is
unrelated to the Software, the license granted hereunder will not terminate
under section (i) of this paragraph due to such counterclaim.

A "Necessary Claim" is a claim of a patent owned by Facebook that is
necessarily infringed by the Software standing alone.

A "Patent Assertion" is any lawsuit or other action alleging direct, indirect,
or contributory infringement or inducement to infringe any patent, including a
cross-claim or counterclaim.

https://github.com/facebook/react/blob/master/PATENTS

It is also possible that my understanding may be wrong. I do not have the time or resources to deal with any patent shenanigans. I do not support software patents. Until the React the PATENTS file is removed, and React is placed under a standard open source license, I won't be using it.

That is just my opinion, feel free to disagree.

workhorsy commented 7 years ago

@btzr-io I'll have to look at preact and polymer more later tonight.

workhorsy commented 7 years ago

I spent a little bit of time making a simple polymer 2.0 and preact website. I had huge problems with polymer:

I don't want to crap all over polymer too much. I think it does have a lot of great ideas, and it is more like what I want out of standard WebComponents.

Anyway. Preact looks like it will be good enough for now. We can always move to something else, or go back to pure JavaScript if this fails. No big deal.

I started on a basic Preact port here: https://github.com/workhorsy/comic_book_reader/tree/preact

btzr-io commented 7 years ago

Nice, I tried to implement translations with preact: https://github.com/Download/preact-i18nline

But there is an issue with preact-i18nline: https://github.com/Download/preact-i18nline/issues/6 https://github.com/Download/i18nline/issues/10

workhorsy commented 7 years ago

Cool. I should probably use preact-i18nline rather than rolling my own translation code.

Download commented 6 years ago

Hi guys. I just noticed this issue because I have picked up work on preact-i18nline again. If you do decide to use it, the coming weeks will be a good time to experiment with it and give some feedback about what is not working right for you, what you think should change or works well etc. So I can incorporate this feedback in my work I'm doing now.

workhorsy commented 6 years ago

Thanks. I haven't had any time to work on the comic book reader. Hopefully someone can continue porting to Preact.

btzr-io commented 6 years ago

@Download awesome, I'm currently working on some components and they have a tooltip assigned with the title attribute, does this work with preact-i18nline or just the text inside the button:

    <button class={style.button} title={label} onClick={action}>
        {...}
       </button>
Download commented 6 years ago

title works I think. There is a whitelist of HTML attributes that get translated as well as the contents of the element.

Download commented 6 years ago

Setting up the project is tricky still at the moment. So let me know if you need help. The docs are a bit out-of-date as well atm.

I'm working on making it simpler to drop into a project and as part of that work will also update the README. Until then I would advice you to keep an eye on https://github.com/backstopmedia/preact-book-example. This is an example project using Preact, created with Preact CLI. I will be pushing a branch there soon with the customizations needed to get it to work with preact-i18nline.