[web] Offer tip for iSCSI and DASD configuration in the UI #500
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ancorgs
changed the title
ancorgs
force-pushed
the
open-sidebar-button
branch
from
7cbd9ab to
5e92c6e
Compare
joseivanlopez
approved these changes
Mar 24, 2023
| // Temporary mini-component with temporary text for March prototype | ||
| const SideBarTip = () => { | ||
| return ( | ||
| <div>If needed, use the <Sidebar.OpenButton>advanced options menu</Sidebar.OpenButton> to configure access to more disks using technologies like iSCSI or DASD (when available).</div> |
There was a problem hiding this comment.
NP: I would remove the last part: "using technologies like iSCSI or DASD (when available)."
There was a problem hiding this comment.
But, it is temporary, so you can give it as it is now.
There was a problem hiding this comment.
I fact, you are right. The text is explanatory enough without that.
By using the Sidebar.OpenButton "subcomponent". It's using a ref by now, but it's needed to research if there is a better way to do it (exploring the useImperativeHanlde[1] hook or any other technique). [1] https://react.dev/reference/react/useImperativeHandle
ancorgs
force-pushed
the
open-sidebar-button
branch
from
71dca45 to
01033c9
Compare
joseivanlopez
approved these changes
Mar 24, 2023
dgdavid
added a commit
that referenced
this pull request
Apr 24, 2023
Because the use case [1] for which it was introduced was already gone: to provide a link for directly opening the sidebar from a text teaching the user where to find page related actions. We can bring back the component if needed, but let's get rid of it now. This commit "reverts" f1a56eb, df85b3c, and f39f65d [1] #500
dgdavid
added a commit
that referenced
this pull request
Apr 24, 2023
Because the use case [1] for which it was introduced was already gone: to provide a link for directly opening the sidebar from a text teaching the user where to find page related actions. We can bring back the component if needed, but let's get rid of it now. This commit "reverts" f1a56eb, df85b3c, and a bit of f39f65d [1] #500
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
It was not always easy to find the options to configure iSCSI and DASD. This is how it looked if there are local devices (or if an iSCSI device was already configured).
If there where not available devices, it was a bit better:
But that solution didn't look much scalable (we could end up with 6 buttons or so when we keep adding technologies). And only iSCSI was mentioned - no reference to DASD, not even in a S390x system.
Solution
First of all, this is a temporary fix. We will re-evaluate options later.
Anyway, now we always show a sentence, even if there are devices.
The link opens the sidebar.
Last but not least, the very same message with the very same behavior is displayed also if there are no devices.
Note not only the messages are temporary, also the implementation itself, as explained in this commit message:
Testing
Tested manually.