Port deprecated Wizard component to new PF5 implementation #53

adamkankovsky · 2023-11-20T13:46:16Z

No description provided.

rvykydal · 2023-11-22T10:01:06Z

@adamkankovsky is the PR ready for review or should I rather wait for tests passing?

adamkankovsky · 2023-11-22T10:03:44Z

@adamkankovsky is the PR ready for review or should I rather wait for tests passing?

I dont have time to test is, so its draft. I look at this tomorow.

rvykydal · 2023-11-23T15:14:38Z

~~Seems like #48 got merged into the commit accidentally somehow.~~

rvykydal

Currently we don't allow moving forward (to a later step - we disable them) on the sidebar (for example from Welcome to Installation Destination). This does not hold with the PR. Seems that what canJumpToStep assured was not ported fully.
We should also create a test for this contidion - I'll do it.

rvykydal · 2023-11-24T10:01:38Z

Currently we don't allow moving forward (to a later step - we disable them) on the sidebar (for example from Welcome to Installation Destination). This does not hold with the PR. Seems that what canJumpToStep assured was not ported fully. We should also create a test for this contidion - I'll do it.

The test PR: #66
Strangely the test passes for me locally on this PR. I can see the issue only when running your PR locally with test/webui_testvm.py fedora-rawhide-boot --rsync
@adamkankovsky can you please rebase the PR on top of the merged test? I'd expect it to pass but let's see :)

The test #66 is passing for me locally.

adamkankovsky · 2023-11-24T12:36:31Z

Currently we don't allow moving forward (to a later step - we disable them) on the sidebar (for example from Welcome to Installation Destination). This does not hold with the PR. Seems that what canJumpToStep assured was not ported fully. We should also create a test for this contidion - I'll do it.

It seems to me that the whole can jump thing was actually about not being able to jump to a step that is hidden, but in the current implementation it is handled a bit differently, so this situation shouldn't arise anyway. Because in the previous implementation of patternfly it was a bit wrong and as a next step the isHidden step was returned

rvykydal · 2023-11-24T15:35:26Z

Currently we don't allow moving forward (to a later step - we disable them) on the sidebar (for example from Welcome to Installation Destination). This does not hold with the PR. Seems that what canJumpToStep assured was not ported fully. We should also create a test for this contidion - I'll do it.

The test PR: #66 Strangely the test passes for me locally on this PR. I can see the issue only when running your PR locally with test/webui_testvm.py fedora-rawhide-boot --rsync @adamkankovsky can you please rebase the PR on top of the merged test? I'd expect it to pass but let's see :)

Ok, so apparently I have problems building updates image for local tests run. Either obsolete image is built using some old .tar.xz or if I remove it I am getting errors with

error: Bad file: /home/rvykydal/work/git/rvykydal/anaconda-webui.1/anaconda-webui-1.20.g3305a966b.tar.xz: No such file or directory

. I guess I must clear something, I'll play with it more, just @KKoukiou FYI.

KKoukiou · 2023-11-24T15:39:06Z

Currently we don't allow moving forward (to a later step - we disable them) on the sidebar (for example from Welcome to Installation Destination). This does not hold with the PR. Seems that what canJumpToStep assured was not ported fully. We should also create a test for this contidion - I'll do it.

The test PR: #66 Strangely the test passes for me locally on this PR. I can see the issue only when running your PR locally with test/webui_testvm.py fedora-rawhide-boot --rsync @adamkankovsky can you please rebase the PR on top of the merged test? I'd expect it to pass but let's see :)

Ok, so apparently I have problems building updates image for local tests run. Either obsolete image is built using some old .tar.xz or if I remove it I am getting errors with
error: Bad file: /home/rvykydal/work/git/rvykydal/anaconda-webui.1/anaconda-webui-1.20.g3305a966b.tar.xz: No such file or directory
. I guess I must clear something, I'll play with it more, just @KKoukiou FYI.

Hm, maybe try starting from a clean repo? git clean -fdx and re-create the updates.img?

KKoukiou

It's obvious now from the new test (thansk @rvykydal) that this regressed the following:

Future steps should be disabled in the sidebar.

rvykydal · 2023-11-27T08:21:54Z

git clean -fdx

@KKoukiou this works, thank you!

src/components/AnacondaWizard.jsx

KKoukiou · 2023-12-06T15:40:00Z

src/components/AnacondaWizard.jsx

-                    window.sessionStorage.setItem("storage-scenario-id", scenarioId);
-                    setStorageScenarioId(scenarioId);
-                }
+    const stepsOrder = useMemo(() => {


What happens if stepsOrder is not memoized? Did you try without?

This is from the part where I tried to disable the next steps. I'm going to go through the code again to see if there's anything like that somewhere.

rvykydal · 2023-12-12T10:13:39Z

src/components/AnacondaWizard.jsx

-
-    const canJumpToStep = (stepId, currentStepId) => {
-        return stepId === currentStepId || isStepFollowedBy(stepId, currentStepId);
+        if (laterStepIdx === -1) {


If I understand it right: "any step is followed by a step which is not found" it seems a bit counterintuitive to me for the function name (and original purpose).

I found a solution where we don't have to change the isStepFollowedBy function.

rvykydal

Looks good to me, thank you!

src/components/AnacondaWizard.jsx

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 2 times, most recently from b9f9a6a to 2966fa0 Compare November 20, 2023 16:57

adamkankovsky changed the title ~~webui: port deprecated Wizard component to new PF5 implementation~~ Port deprecated Wizard component to new PF5 implementation Nov 21, 2023

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 4 times, most recently from 754e3bc to 7bae548 Compare November 23, 2023 14:38

adamkankovsky requested review from KKoukiou and rvykydal November 23, 2023 14:58

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 7bae548 to 84dd2b1 Compare November 23, 2023 14:59

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 7 times, most recently from 74b8c7c to 9c7cd19 Compare November 23, 2023 21:46

rvykydal previously requested changes Nov 24, 2023

View reviewed changes

rvykydal self-requested a review November 24, 2023 10:01

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 9c7cd19 to 6c58d4a Compare November 24, 2023 12:31

KKoukiou reviewed Nov 24, 2023

View reviewed changes

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 2 times, most recently from db5b14e to 581ce4d Compare November 29, 2023 15:37

KKoukiou force-pushed the webui-porting-deprecated-wizard branch from 581ce4d to 5d3c1c0 Compare December 5, 2023 14:29

github-advanced-security bot found potential problems Dec 5, 2023

View reviewed changes

src/components/AnacondaWizard.jsx Fixed Show fixed Hide fixed

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 5d3c1c0 to 9d91f3d Compare December 6, 2023 12:45

KKoukiou reviewed Dec 6, 2023

View reviewed changes

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 9d91f3d to 502583c Compare December 7, 2023 08:49

rvykydal reviewed Dec 12, 2023

View reviewed changes

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 3 times, most recently from eaf9a20 to c852276 Compare December 12, 2023 12:54

rvykydal approved these changes Dec 12, 2023

View reviewed changes

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch 2 times, most recently from cf537a7 to 9af1c46 Compare December 12, 2023 14:17

github-advanced-security bot found potential problems Dec 12, 2023

View reviewed changes

src/components/AnacondaWizard.jsx Fixed Show fixed Hide fixed

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 9af1c46 to e8ad6f1 Compare December 12, 2023 14:20

adamkankovsky requested a review from KKoukiou December 12, 2023 14:23

KKoukiou reviewed Dec 12, 2023

View reviewed changes

src/components/AnacondaWizard.jsx Outdated Show resolved Hide resolved

adamkankovsky added 2 commits December 12, 2023 16:32

Port deprecated Wizard component to new PF5 implementation

d5b263d

Renaming the transition function to not be called as a callback

ddaf543

adamkankovsky force-pushed the webui-porting-deprecated-wizard branch from 3524bd0 to ddaf543 Compare December 12, 2023 15:35

KKoukiou merged commit 755ead3 into rhinstaller:main Dec 12, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port deprecated Wizard component to new PF5 implementation #53

Port deprecated Wizard component to new PF5 implementation #53

adamkankovsky commented Nov 20, 2023

rvykydal commented Nov 22, 2023

adamkankovsky commented Nov 22, 2023

rvykydal commented Nov 23, 2023 •

edited

Loading

rvykydal left a comment •

edited

Loading

rvykydal commented Nov 24, 2023 •

edited

Loading

adamkankovsky commented Nov 24, 2023 •

edited

Loading

rvykydal commented Nov 24, 2023

KKoukiou commented Nov 24, 2023

KKoukiou left a comment

rvykydal commented Nov 27, 2023

KKoukiou Dec 6, 2023

adamkankovsky Dec 7, 2023

rvykydal Dec 12, 2023

adamkankovsky Dec 12, 2023

rvykydal left a comment

Port deprecated Wizard component to new PF5 implementation #53

Port deprecated Wizard component to new PF5 implementation #53

Conversation

adamkankovsky commented Nov 20, 2023

rvykydal commented Nov 22, 2023

adamkankovsky commented Nov 22, 2023

rvykydal commented Nov 23, 2023 • edited Loading

rvykydal left a comment • edited Loading

Choose a reason for hiding this comment

rvykydal commented Nov 24, 2023 • edited Loading

adamkankovsky commented Nov 24, 2023 • edited Loading

rvykydal commented Nov 24, 2023

KKoukiou commented Nov 24, 2023

KKoukiou left a comment

Choose a reason for hiding this comment

rvykydal commented Nov 27, 2023

KKoukiou Dec 6, 2023

Choose a reason for hiding this comment

adamkankovsky Dec 7, 2023

Choose a reason for hiding this comment

rvykydal Dec 12, 2023

Choose a reason for hiding this comment

adamkankovsky Dec 12, 2023

Choose a reason for hiding this comment

rvykydal left a comment

Choose a reason for hiding this comment

rvykydal commented Nov 23, 2023 •

edited

Loading

rvykydal left a comment •

edited

Loading

rvykydal commented Nov 24, 2023 •

edited

Loading

adamkankovsky commented Nov 24, 2023 •

edited

Loading