[web] Change the location of page options #545
Conversation
Basically used by the Sidebar, which will stop holding "Page" or "Contextual" actions. Another slot will be created for them in order to make these actions more discoverable.
Based in the PatternFly Dropdown component[1] and intended for displaying the actions related to the current page in an attempt to make them more discoverable. [1] https://www.patternfly.org/v4/components/dropdown
And stop using the somehow "deprecated" PageOptions.
Moving the actions to its own NetworkPageOptions page and stop using the "deprecated" PageOptions to show them in the Sidebar.
Now that we are going to stop teleporting links to the Sidebar and start using another location for placing the page related links, the "Change product" action will always be displayed in the Sidebar since, at least initially, we don't want to relate actions only to the Overview.
It'll become no longer needed
No longer needed since we've decided to stop teleporting page related actions to make them more discoverable.
dgdavid
changed the title
Pull Request Test Coverage Report for Build 4794910727
💛 - Coveralls |
Forgotten at dd0065b
| <ContextualActions.Item | ||
| key="iscsi-link" | ||
| href={href} | ||
| description="Connect to iSCSI targets" |
There was a problem hiding this comment.
With this new approach, it's a lot of easy to add a description for each menu entry and help the users to quickly identify that it's really the action they want to choose. That's why I added this description prop here after talking about that with @ancorgs. Please, check it and propose a better description if you have one in mind. The idea is to avoid using only the acronyms as much as possible too.
There was a problem hiding this comment.
Just to be sure this leads only to new connections or does it allow also to modify existing/delete connections?
There was a problem hiding this comment.
Actually, it leads to the iSCSI management page. The idea to use the "Create" word it's based in the fact that probably it's the "most wanted" action. As said, it's a hint to avoid having only the acronym as a menu option (which is not wrong per se), but any better suggestion as an entry description is more than welcome.
There was a problem hiding this comment.
In that case I think "Manager connections to iSCSI targets" sounds better to me.
There was a problem hiding this comment.
I see the value in "Connect to iSCSI Targets" even if it's not precise. It's true it takes you to a place in which you can connect, disconnect, discover, check the iBFT configuration and even more things I may be overlooking. But the first time you are looking for that page is because you want to connect to some target.
| return ( | ||
| <ContextualActions> | ||
| <ContextualActions.Group | ||
| label="Configure" |
There was a problem hiding this comment.
Instead of having a lot of "similar actions" like "Configure DASD", "Configure iSCSI", and "Configure FCoE" for example, we decided to group them under the "Configure" group. Now can be difficult to see the benefit of grouping page options, but think that most probably it will grow sooner than later. Anyway we can re-evalute it if needed an introduce groups later, but please keep in mind that its a bit confusing having a bunch of "Configure ACRONYM" options in a row.
| <ContextualActions.Item | ||
| key="dasd-link" | ||
| href={href} | ||
| description="Manage and format" |
There was a problem hiding this comment.
As in the iSCSI link,
With this new approach, it's a lot of easy to add a description for each menu entry and help the users to quickly identify that it's really the action they want to choose. That's why I added this description prop here after talking about that with @ancorgs. Please, check it and propose a better description if you have one in mind. The idea is to avoid using only the acronyms as much as possible too.
| const Toggler = ({ onClick }) => { | ||
| return ( | ||
| <Button onClick={onClick} variant="plain"> | ||
| <Icon name="expand_more" /> |
There was a problem hiding this comment.
I actually like the "+" icon, which for me has a lot of sense (more page actions). But since I thought it might be a bit controversial... I opted for using a "dropdown arrow" like one. Anyway, see how they look side by side
| "Expand more" icon | "Add" icon |
|---|---|
 |
 |
Either way, there are plenty of them at https://fonts.google.com/icons. Have a look to see if you find one that fits better.
There was a problem hiding this comment.
I like both, and I see the down arrow more generic. I would it for now.
There was a problem hiding this comment.
I prefer arrow instead of + as + invokes to me adding something like new storage. And it is not generic to all usage and we should be really be consistent, so user when get familiar with one page will be familiar with all of them.
There was a problem hiding this comment.
As said, I also like the arrow. But I am asking myself if the icon is dicoverable enough. At first sight it looks to me like a button for minimizing or something similar (maybe it is my fault as user). Moreover, how can we refer to it? I mean, in the devices selector popup we have a sentence for suggesting the user to configure more devices if needed. Do we need to keep that tip? If so, how?
There was a problem hiding this comment.
BTW, how do you see to add a button? E.g., "More options V", with good styles to integrate it nicely with the sidebar.
There was a problem hiding this comment.
I do not think it is needed such verbosity. But if you want, we can go ahead and fill the header with words 😃
There was a problem hiding this comment.
Maybe I'd use the tooltip here.
There was a problem hiding this comment.
At first sight it looks to me like a button for minimizing or something similar
That's one of the reason why I prefer another icon. But I do not think, honestly, this would be a big issue at this time. What is more, I'd like to have feedback for real users. I.e., start simple and react to the audience feedback. The worst can happen is to have a user discovering more options becuase wanted to minimize the storage page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
For not displaying it when the WiFi scan is not supported. See #545 (comment)
The option for configuring additional devices should be discoverable enough now and, hopefully, the user does not need to be taught anymore about where to find options related to a specific page. What is more, it's no longer true that such actions can be done from the Sidebar.
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
dgdavid
force-pushed
the
contextual-actions-2nd-approach
branch
from
301b3de to
a492944
Compare
dgdavid
changed the title
Because the component is more related to a PageMenu or PageOptions and not to a "contextual actions".
dgdavid
force-pushed
the
contextual-actions-2nd-approach
branch
from
63772a3 to
6a89990
Compare
Problem
Since page actions (also known as "Contextual actions") were introduced, there have been complaints regarding their "discoverability" from within and outside the team.
It seems that there is too much subtlety in the navigation (and, admittedly, I like that fact) to make it easy for the users to discern when they are in a new page and, thus, the Hamburguer Menu icon (aka Sidebar) may contain different options.
The situation can be further complicated now that we plan to add a global notification mark alongside the Hamburguer Menu icon for trying to catch users' attention in a no disruptive way when there is something that might be interesting for them but not critical or urgent enough for blocking the UI with a Popup.
Click to show/hide few screenshots
Solution
To extract the page related actions from the Sidebar and add a more evident element in the header to reveal them.
Click to show/hide more screenshots
Side effects
These changes entail the removal of the
PageOptionscomponent introduced at #462 as well as theSidebar.OpenButtonintroduced at #500Testing