Remove dependency on WebView

[j-lum]

The help command is the only command that relies on WebView, a dependency that weighs in at around 70MB, more than quadrupling the size of the executable jar. As there are plans to customize the jars for each individual student during the practical examination, this overhead can lead to further problems (bandwidth in the lecture hall, storage space, etc).

Let's remove the dependency on WebView by changing the help command to output a URL to the user guide instead.

[CanIHasReview-bot]

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

[CanIHasReview-bot]

v1

@j-lum submitted v1 for review.

• [v1 1/1] Remove dependency on WebView

(:books: Archive)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/27/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[damithc]

There is a Travis error though. Fix and do a v2?

[j-lum]

Completely forgot to show whitespace on my home setup.

[CanIHasReview-bot]

v2

@j-lum submitted v2 for review.

• [v2 1/1] Remove dependency on WebView

(:books: Archive) (:chart_with_upwards_trend: Interdiff between v1 and v2) (:chart_with_upwards_trend: Range-Diff between v1 and v2)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/27/2/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[j-lum]

I missed out some things in the initial commit.

• Remove HelpWindow.adoc
• Remove Gradle tasks that copy HelpWindow into resource folder

[CanIHasReview-bot]

v3

@j-lum submitted v3 for review.

• [v3 1/1] Remove dependency on WebView

(:books: Archive) (:chart_with_upwards_trend: Interdiff between v2 and v3) (:chart_with_upwards_trend: Range-Diff between v2 and v3)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/27/3/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[damithc]

Eh, I thought that the HelpWindow also doubles as an example of how to write and show a dialog box. May want to check with @damithc about that.

Yes, that's a good idea. @j-lum can?

Also, I'm not sure why https://github.com/se-edu/addressbook-level3/blob/master/docs/UserGuide.adoc was chosen as the URL compared to https://se-education.org/addressbook-level3/UserGuide.html which is usually what users would expect to see as a user guide. In fact, before se-edu/addressbook-level4@6ec9b68 that was the URL we used.

Yes, it is better to use the html link.

[j-lum]

Actually replacing the original webview with a label looks like the path of least resistance so let's do that for now.

I linked the adoc because I'm worried that some students won't be on netlify. Will change the official copy to link to html page.

[damithc]

Actually replacing the original webview with a label looks like the path of least resistance so let's do that for now.

Yes, let's do that. Plus, it gives students an example of opening child windows.

I linked the adoc because I'm worried that some students won't be on netlify. Will change the official copy to link to html page.

Netlify preview is optional but auto-publishing (via Travis) is a requirement i.e., the html link will work in all projects.

[damithc]

@j-lum Let's resume this.

[damithc]

Let's wrap this up in and do a release by Thursday. Students are expected to download AB3 jar file starting from this Friday.

[j-lum]

I iterated through a bunch of layouts for HelpWindow and eventually settled on this:

Other things I've tried are: Using an immutable TextField to display the URL results in having some really funky code to resize the TextField.

Just the plain Label is not helpful as the user cannot copy the label.

Hyperlinks are a mess in JavaFX.

Images in buttons are a bit overkill just for a clipboard icon so I went with the unicode emoji.

[j-lum]

Eh CIHR bot failed with an unknown error with an ID: job-090a7c34-103a-494a-b190-6d67961f24bb so I'm just manually flipping the status for now.

[canihasreview[bot]]

v4

@pyokagan submitted v4 for review.

• [v4 1/1] Remove dependencies on WebView

(:books: Archive) (:chart_with_upwards_trend: Interdiff between v3 and v4) (:chart_with_upwards_trend: Range-Diff between v3 and v4)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/27/4/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[pyokagan]

Eh CIHR bot failed with an unknown error with an ID: job-090a7c34-103a-494a-b190-6d67961f24bb so I'm just manually flipping the status for now.

The logs say:

Running git ["push","-f","origin","+refs/pr/27/4/*:refs/pr/27/4/*"]
remote: fatal error in commit_refs
To https://github.com/se-edu/addressbook-level3.git
 ! [remote rejected] refs/pr/27/4/base -> refs/pr/27/4/base (failure)
 ! [remote rejected] refs/pr/27/4/head -> refs/pr/27/4/head (failure)
 ! [remote rejected] refs/pr/27/4/interdiff -> refs/pr/27/4/interdiff (failure)
 ! [remote rejected] refs/pr/27/4/rangediff -> refs/pr/27/4/rangediff (failure)
error: failed to push some refs to 'https://github.com/se-edu/addressbook-level3.git'

Disabling branch protection seems to solve it, but the generic remote: fatal error in commit_refs error message from GitHub makes me think it's a bug on GitHub's side. Unfortunately I'm having trouble reproducing it with a new repo :-(

[pyokagan]

This was added in 1488051, with obvious reference to the processResources task, so if the processResources task is being removed it makes sense to remove this as well.

Also, as the commit 1488051 shows, we should probably be removing the "HelpWindowTest" troubleshooting entry in Testing.adoc :-P

[j-lum]

I was under the impression that media was brought in because it was a dependency that web relies on as the JavaFX plugin pulls media in when web is required.

From https://github.com/openjfx/javafx-gradle-plugin/blob/master/src/main/java/org/openjfx/gradle/JavaFXModule.java :

public enum JavaFXModule {
    BASE,
    GRAPHICS(BASE),
    CONTROLS(BASE, GRAPHICS),
    FXML(BASE, GRAPHICS),
    MEDIA(BASE, GRAPHICS),
    SWING(BASE, GRAPHICS),
    WEB(BASE, CONTROLS, GRAPHICS, MEDIA);
//...
}

I could go to the fxml and manually change the version of JavaFX to 11 to fix the warning.

Your exception seems to be problem with emojis: https://github.com/javafxports/openjdk-jfx/issues/287 . I'll purge the clipboard😢. It works fine on my VM though, so it might be a distro issue that I'm not really willing to fix.

[canihasreview[bot]]

v5

@j-lum submitted v5 for review.

• [v5 1/1] Remove dependencies on WebView

(:books: Archive) (:chart_with_upwards_trend: Interdiff between v4 and v5) (:chart_with_upwards_trend: Range-Diff between v4 and v5)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/27/5/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[pyokagan]

I was under the impression that media was brought in because it was a dependency that web relies on as the JavaFX plugin pulls media in when web is required.

From https://github.com/openjfx/javafx-gradle-plugin/blob/master/src/main/java/org/openjfx/gradle/JavaFXModule.java :

public enum JavaFXModule {
    BASE,
    GRAPHICS(BASE),
    CONTROLS(BASE, GRAPHICS),
    FXML(BASE, GRAPHICS),
    MEDIA(BASE, GRAPHICS),
    SWING(BASE, GRAPHICS),
    WEB(BASE, CONTROLS, GRAPHICS, MEDIA);
//...
}

Ah OK, that sounds good.

I could go to the fxml and manually change the version of JavaFX to 11 to fix the warning.

Yes, that sounds good for this PR. However, if students use Gluon SceneBuilder in the future they would get this issue again, so it might be good to upgrade to JavaFX 11.0.1 in another commit.

Your exception seems to be problem with emojis: javafxports/openjdk-jfx#287 . I'll purge the clipboardcry. It works fine on my VM though, so it might be a distro issue that I'm not really willing to fix.

Great, it works for me now :+1:

[damithc]

Great. Almost there. @j-lum can merge this and do a release?