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

storage: move 'Modify storage' to the header kebab #444

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

KKoukiou
Copy link
Contributor

@KKoukiou KKoukiou commented Sep 19, 2024

Merge CockpitStorageIntegration and ModifyStorage files into one, having the "Modify storage" button split from the cockpit integration page implementation was slightly weird.

Also stop utilizing the 'isFormDisabled' in the
CockpitStorageIntegration, we don't disable the form anyhow from within cockpit-storage mode, so this was causing unnecessary complexity in the code.

Related to: https://issues.redhat.com/browse/INSTALLER-3972

@rvykydal
Copy link
Contributor

@KKoukiou I've played with the patch in my local VM and hit an issue when I

  • have 2 disks
  • create 1GB /boot 100MB biosboot, rest / in Storage Editor
  • then go back to installation
  • go to mount point assignment
  • the UI is blinking in some update loop perhaps
    webui_2disks_mpa_flashing.webm
    May be worth trying to reproduce on your side.

Merge CockpitStorageIntegration and ModifyStorage files into one, having
the "Modify storage" button split from the cockpit integration page implementation
was slightly weird.

Also stop utilizing the 'isFormDisabled' in the
CockpitStorageIntegration, we don't disable the form anyhow from within
cockpit-storage mode, so this was causing unnecessary complexity in the
code.
@KKoukiou
Copy link
Contributor Author

@KKoukiou I've played with the patch in my local VM and hit an issue when I

* have 2 disks

* create 1GB /boot 100MB biosboot, rest / in Storage Editor

* then go back to installation

* go to mount point assignment

* the UI is blinking in some update loop perhaps
  [webui_2disks_mpa_flashing.webm](https://github.com/user-attachments/assets/d0195e1e-3ef3-49c1-9de0-0ac8f8a60ac8)
  May be worth trying to reproduce on your side.

Not sure if this bug is related to this PR (probably not) but I found what the issue was and sent two commits on top.

Copy link
Contributor

@rvykydal rvykydal left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@KKoukiou KKoukiou merged commit ef25a33 into rhinstaller:main Sep 20, 2024
9 checks passed
@KKoukiou KKoukiou deleted the modify-storage-header-move branch September 20, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants