Skip to content

Downloading route content (Github, zip) #903

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.

Sign up for GitHub

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

Jump to bottom
Merged
cjakeman merged 45 commits into from
Aug 27, 2024
Merged

Downloading route content (Github, zip) #903

cjakeman merged 45 commits into from
Aug 27, 2024

Conversation

@sweiland-openrails
Copy link
Contributor

@sweiland-openrails sweiland-openrails commented Dec 21, 2023 •
edited by twpol
Loading

Discussion: http://www.elvastower.com/forums/index.php?/topic/31918-downloading-content-for-open-rails
Roadmap: https://trello.com/c/1YXKK7N5/164-publishing-and-downloading-content

I've created within the Main menu a separate window "Download Content". So that it can be enlarged when more download options would to be added (trains, consists, etc.). Just like the Resume/Replay.

image

Wishes for the second phase:

  • add uninstall button
  • add info button, starts webpage
  • replace hardcoded GitHub URL's by downloading a file with it from GitHub Open Rails
  • add check for diskspace and tell how much diskspace is used by the download. "Download" might not be the right verb, "Install" to be used?
  • progress bar, there is no such thing as a callback for every MB downloaded from the repository Clone. I might be able to get the total amount Mb to be downloaded versus the already downloaded MB in a progress bar. And have the download process running in a different thread.
  • load the just downloaded route in the menu as the to be started route. Or even start the route.
  • add a update button and beforehand show what's get updated.

@sweiland-openrails sweiland-openrails added the enhancement New feature or request label Dec 21, 2023
@sweiland-openrails sweiland-openrails marked this pull request as draft December 21, 2023 15:21
@sweiland-openrails sweiland-openrails changed the title first phase downloading from Github Downloading route content from Github Dec 24, 2023
@sweiland-openrails sweiland-openrails changed the title Downloading route content from Github Downloading route content (Github, zip) Dec 27, 2023
sweiland-openrails and others added 2 commits August 11, 2024 10:35
@sweiland-openrails @cjakeman
Update Source/Documentation/Manual/start.rst
86e883e 
Co-authored-by: Chris Jakeman <github@jakeman.plus.com>
@sweiland-openrails @cjakeman
Update Source/Documentation/Manual/start.rst
4a4b032 
Co-authored-by: Chris Jakeman <github@jakeman.plus.com>
@sweiland-openrails sweiland-openrails marked this pull request as ready for review August 22, 2024 07:45
@sweiland-openrails
Copy link
Contributor Author

sweiland-openrails commented Aug 22, 2024

hi Chris, to my opinion this enhancement is ready for a final review and merge. Do you agree?

@cjakeman
Copy link
Contributor

cjakeman commented Aug 22, 2024

Hi Siebren,

hi Chris, to my opinion this enhancement is ready for a final review and merge. Do you agree?

It all seems to be working well. I spoke with James about it on Saturday and we want to move Options > Content into a tab on your new form before it gets released. All the content would then be accessible via the "Content" button.

I'll post some more at the weekend to see what your thoughts are. James and I are happy to do some of the work if necessary to get this released.

Best wishes,

Chris

cjakeman
cjakeman reviewed Aug 23, 2024
View reviewed changes
cjakeman
cjakeman reviewed Aug 23, 2024
View reviewed changes
@cjakeman
Copy link
Contributor

cjakeman commented Aug 24, 2024

Hi Siebren,

I'll post some more at the weekend to see what your thoughts are. James and I are happy to do some of the work if necessary to get this released.

I'm very pleased that you have provided all this new functionality. It's going to make "Getting Started" so much easier for a newcomer.

Our main concern for this PR has been where the data for the installed routes is kept. I see that you have moved it from AppData\Roaming\OpenRails\Settings into the Registry, so that's all sorted. Thanks.

I've now submitted a review with a few minor points.

Once this PR is merged, I would like to discuss another PR for changes prior to releasing as v1.6:

  • We would like to see the Options > Content tab merged into your new form - something like this:

image

  • Any selection of a route should change the selection in the main form too - a behaviour that has been overlooked for years.

  • The new form seems to be compressed vertically. James has offered to tidy it up to match the main form better.

  • Perhaps we could discuss the choice of information shown in DownloadContent. For example, the author or the date might be more useful than the URL.

  • Perhaps we could find an alternative to the Info button to present the data without requiring a browser.

If you would be willing to undertake another PR along these lines, that would be terrific. If not, then perhaps I can help out.

Best wishes,

Chris

cjakeman
cjakeman requested changes Aug 24, 2024
View reviewed changes
Copy link
Contributor

@cjakeman cjakeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Siebren,

Please remove the redundant statement about Start from the Manual.

The DownloadContent form has a resize control which does nothing. Please remove that also.

Then I'll approve and merge it.

Thanks,

Chris

sweiland-openrails and others added 5 commits August 26, 2024 20:31
@sweiland-openrails @cjakeman
Update Source/Documentation/Manual/start.rst
408cb1d 
Sure, forgot this one, thanks.

Co-authored-by: Chris Jakeman <github@jakeman.plus.com>
@sweiland-openrails
Typo: changed mainForm.LoadFolderListWhithoutTask(); into mainForm.Lo…
7b5496d 
…adFolderListWithoutTask();
@sweiland-openrails
Undo previous commit
c06f5d0
@sweiland-openrails
remove resize control from Download Content form
c2b7f1c 
disable maximize button on Download Content form
@sweiland-openrails
Merge branch 'downloadContent' of https://github.com/sweiland-openrai…
25cc886 
…ls/openrails into downloadContent
cjakeman
cjakeman approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@cjakeman cjakeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Siebren,

Thanks for resolving those issues,

Approved and merged,

Chris

@cjakeman cjakeman merged commit 814391e into openrails:master Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjakeman cjakeman cjakeman approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request for-unstable Special label to include a pull request in the Unstable Version

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sweiland-openrails @cjakeman