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.

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

Changed Caption of last -Next button in Windows InstallationWizard to -Install button #48537

Merged

Conversation

AbhaySingh004
Copy link
Contributor

closes #48521
Bug Description-
PR-Bug-Report

Here the Next button should be replaced with Install

the Windows-Installer-Wizard shows the Install button on the ReadyPage of other software
as provided attachments in #48521

Bug Fix-

-now to solve this bug

in builder.iss inno-Setup-Script file the disablereadypage=yes is not making Installation-Wizard reach its readypage .

PR-Builder

so to fix this I gone through Setup guide of Inno-Setup-Script and found the solution
link-

like show here in attachment below

PR-Inno

@DilumAluthge DilumAluthge added the system:windows Affects only Windows label Feb 5, 2023
@AbhaySingh004
Copy link
Contributor Author

https://jrsoftware.org/ishelp/topic_setup_disablereadypage.htm
this is the link of Inno-Setup guide.
which shows how to handle DisableReadyPage=yes instruction to get install button currentWizardform

@giordano
Copy link
Contributor

giordano commented Feb 5, 2023

Hi, thanks for your contribution. Can you please give the pull request a more descriptive title?

@AbhaySingh004 AbhaySingh004 changed the title Windows-InstallationWizard-Form-BugFix Changed Caption of last -Next button in Windows InstallationWizard to -Install button Feb 5, 2023
@AbhaySingh004
Copy link
Contributor Author

Hi, thanks for your contribution. Can you please give the pull request a more descriptive title?

thank you for your guidance. I have edited the title
Do you think it's good now? Or should I be more descriptive?

@@ -147,6 +147,12 @@ end;

procedure CurPageChanged(CurPageID: Integer);
begin
; Fixing bug in git Issue #48521

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comments would better describe the behaviour of the following code directly. Something like: change button text from next to install when there are no more options.

yes Surely, thank you for the suggestion.
i did changed the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad for not expressing myself clearly.

Delete line 150 ~ line 155. Line 160 can replace them completely.

yes got it!
i hope its ready to merge now.

@AbhaySingh004 AbhaySingh004 requested review from inkydragon and removed request for staticfloat and davidanthoff February 5, 2023 13:12
@@ -147,6 +147,12 @@ end;

procedure CurPageChanged(CurPageID: Integer);
begin
; change button text from "next" to "install" when ReadyPage is disabled.
if CurPageID = wpSelectProgramGroup then
Copy link
Member

Choose a reason for hiding this comment

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

For example, if the new last pre-installation wizard page is the Select Program Group page:

Our last page is wpSelectTasks.

if CurPageID = wpSelectProgramGroup then
WizardForm.NextButton.Caption := SetupMessage(msgButtonInstall)
else
WizardForm.NextButton.Caption := SetupMessage(msgButtonNext);
Copy link
Member

Choose a reason for hiding this comment

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

This will override the button "Finish" on the final page.

contrib/windows/build-installer.iss Show resolved Hide resolved
@AbhaySingh004
Copy link
Contributor Author

i commited @inkydragon the changes you suggested.

@inkydragon
Copy link
Member

inkydragon commented Feb 5, 2023

I mean, just add a new branch for that case.
The if-else code block can be removed.


For other reviewer

You don't need to build julia, a new folder and a fake exe is enough to complete the package test.

# In JuliaLang/julia/

# download innosetup
make win-extras

# make fake binary dir
mkdir -p julia-innosetup/bin/
# make fake exe
echo 1 > julia-innosetup/bin/julia.exe

Run this in MSYS2, Or you may need to manually combine the full path, for example by packing it in cmd

# pack installer
MSYS2_ARG_CONV_EXCL='*'  ./dist-extras/inno/iscc.exe /DAppVersion=1.10.0-DEV /DSourceDir="`cygpath -w -a ./julia-innosetup`" /DRepoDir="`cygpath -w -a .`" /O"`cygpath -w -a .`"  "`cygpath -w -a ./contrib/windows/build-installer.iss`"

@AbhaySingh004
Copy link
Contributor Author

AbhaySingh004 commented Feb 5, 2023

hey @inkydragon, I am new to the open source community.
I want some guidance on this
Just a query for a new branch, do I have to create a new Pull Request?

@inkydragon
Copy link
Member

My bad for not expressing myself clearly.

Delete line 150 ~ line 155.
Line 160 can replace them completely.

@@ -145,11 +145,15 @@ begin
end;
end;


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

procedure CurPageChanged(CurPageID: Integer);
begin
case CurPageID of
wpWelcome: WizardForm.Color := WizardForm.WelcomePage.Color;
wpFinished: WizardForm.Color := WizardForm.FinishedPage.Color;

;change button text from "next" to "install" when ReadyPage is disabled.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
;change button text from "next" to "install" when ReadyPage is disabled.
// change button text from "next" to "install" when ReadyPage is disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank You So much @inkydragon for helping me out with this.
I committed the changes you suggested, is it alright now?

Copy link
Member

@inkydragon inkydragon left a comment

Choose a reason for hiding this comment

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

I have tested this pr locally and it looks fine.
cc: @staticfloat

image

@inkydragon inkydragon added the building Build system, or building Julia or its dependencies label Feb 6, 2023
@staticfloat
Copy link
Member

Great, thanks! Welcome to the Julia community, @AbhaySingh004!

@staticfloat staticfloat merged commit 8860d95 into JuliaLang:master Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The last Next button in Windows installer should say Install instead
5 participants