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.

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

Customized Welcome After Pages and Conclusion Pages #590

Merged
merged 39 commits into from Jan 11, 2023
Merged

Customized Welcome After Pages and Conclusion Pages #590

merged 39 commits into from Jan 11, 2023

Conversation

ryanskeith
Copy link
Contributor

@ryanskeith ryanskeith commented Dec 7, 2022 •
edited

Description

  • This PR enables the adding of extra pages to the Windows GUI installer. These extra pages can display text, links, and images. Such pages can be useful in providing additional instructions, guidance, or promotional materials for end-users before they begin using the application they just installed.
  • This addresses issues brought up in PR Display extra pages in Windows installers #582.

Checklist - did you ...

  • [ x ] Add a file to the news directory (using the template) for the next release's release notes?
  • [ x ] Add / update outdated documentation?

Screen shots

Screen Shot 2022-12-07 at 12 43 38 PM

@ryanskeith ryanskeith requested a review from a team as a code owner December 7, 2022 20:39
@conda-bot
Copy link
Contributor

We require contributors to sign our Contributor License Agreement and we don't have one on file for @ryanskeith.

In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.

@jezdez
Copy link
Member

jezdez commented Dec 7, 2022

@conda-bot check

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 7, 2022
@pseudoyim pseudoyim mentioned this pull request Dec 7, 2022
Closed
3 tasks
@dbast
Copy link
Member

dbast commented Dec 8, 2022

Requires #589 to be merged first due to unrelated CI errors.

@dbast
Copy link
Member

dbast commented Dec 8, 2022

Also requires to locally run scripts/make_docs.py and commit the resulting updated CONSTRUCT.md file.

jaimergp
jaimergp requested changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Super cool to see this happening. One more step towards the fork re-integration! Thanks for the submission.

I have two main comments about the implementation:

  • I'd rather see the custom welcome and conclusion pages as replacement pages, not additional ones, by default. Of course, the custom page can define again the MUI_*_PAGE macros to re-insert the replaced pages, but this way users actually have a way to remove the default pages.
  • I'd rather see how we reuse extra_files to handle the temporary files.

These are not strong opinions and I'll be happy to discuss them, but this is how I had envisioned the initial implementation.

constructor/osxpkg.py Show resolved Hide resolved
constructor/osxpkg.py Show resolved Hide resolved
constructor/winexe.py Show resolved Hide resolved
constructor/nsis/main.nsi.tmpl Outdated Show resolved Hide resolved
constructor/winexe.py Show resolved Hide resolved
constructor/nsis/main.nsi.tmpl Show resolved Hide resolved
constructor/nsis/main.nsi.tmpl Outdated Show resolved Hide resolved
dbast and others added 9 commits December 13, 2022 16:53
jaimergp
jaimergp reviewed Dec 15, 2022
View reviewed changes
constructor/winexe.py Outdated Show resolved Hide resolved
ryanskeith and others added 2 commits January 4, 2023 12:10
@ryanskeith @jaimergp
Update news/590-windows-installer-enhancement
a13c3de 
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@ryanskeith @jaimergp
Update constructor/osxpkg.py
3d0d3bb 
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@ryanskeith ryanskeith mentioned this pull request Jan 4, 2023
Closed
1 task
@ryanskeith
Copy link
Contributor Author

@jaimergp changes from PR #594 have been merged when you would like to take a gander.

jaimergp
jaimergp reviewed Jan 9, 2023
View reviewed changes
constructor/winexe.py Outdated Show resolved Hide resolved
@jaimergp
Copy link
Contributor

jaimergp commented Jan 9, 2023

@ryanskeith Some more comments that need addressing, but we are getting very close to a mergeable state :D I would like to play with this PR a bit to get a feeling on how it could work for other projects before merging, but I think it all can happen within the week!

ryanskeith and others added 4 commits January 9, 2023 16:00
@ryanskeith @jaimergp
Update examples/customized_welcome_conclusion/construct.yaml
66eeb31 
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@ryanskeith @jaimergp
Update news/590-windows-installer-enhancement
e148b6a 
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@ryanskeith @jaimergp
Update constructor/winexe.py
7cceea5 
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@ryanskeith
Adding in comments to explain example custom pages.
cae3969
@ryanskeith
Copy link
Contributor Author

@jaimergp Comments added to requested file.

jaimergp
jaimergp reviewed Jan 11, 2023
View reviewed changes
examples/customized_welcome_conclusion/custom_welcome.nsi

${NSD_CreateLink} 10u 55u 200u 10u "https://github.com/conda"
Pop $InstallationAfterWelcomeLink
${NSD_OnClick} $InstallationAfterWelcomeLink LaunchLinkOne
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this link work? The function is only defined later (in the conclusion file), so I am not sure NSIS knows about it now (maybe it does). I just want to know whether this link in the welcome page works. Can you confirm? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it seemed to work in our tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome. Thanks!

@jaimergp
Copy link
Contributor

Thanks @ryanskeith! I fixed the conflicts and added one more (last?) question. Can you resync the docs (python scripts/make_docs.py)? I am going to merge soon and play with it once we are done. <3

@dbast
Copy link
Member

dbast commented Jan 11, 2023

docs should be resynced

@jaimergp
Copy link
Contributor

Thanks! I'll merge as soon as everything's green.

@jaimergp jaimergp merged commit 12899a5 into conda:main Jan 11, 2023
@ryanskeith ryanskeith deleted the customize_welcome_conclusion branch January 11, 2023 20:10
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Jan 12, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaimergp jaimergp jaimergp approved these changes

@dbast dbast dbast approved these changes

Assignees
No one assigned
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
🧭 Planning
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ryanskeith @conda-bot @jezdez @dbast @jaimergp