feat: [M3-7345] – Unrecommended Linode configuration indicators on VPC Detail page #9914
Conversation
…mended configuration notice displayed (unconditionally for now) on VPC Detail page; fix visual defect in Storybook for Notices > Usage
… on SubnetLinodeRow, updates
dwiley-akamai
requested review from
hana-akamai and
coliu-akamai
and removed request for
a team
| @@ -79,7 +79,7 @@ export interface NoticeProps extends Grid2Props { | |||
| - Appear within the page or modal | |||
| - Might be triggered by user action | |||
| - Typically used to alert the user to a new service, limited availability, or a potential consequence of the action being taken | |||
| - Consider using a [Dismissible Banner](/docs/components-notifications-dismissible-banners--beta-banner) if it’s not critical information | |||
| - Consider using a [Dismissible Banner](/docs/components-notifications-dismissible-banners--beta-banner) if it’s not critical information | |||
There was a problem hiding this comment.
This fixes this issue at design.linode.com:
| const [ | ||
| unrecommendedConfigurationPresent, | ||
| setUnrecommendedConfigurationPresent, | ||
| ] = React.useState(false); |
There was a problem hiding this comment.
Grabbing all the configs in this component could get expensive and messy, so this gets passed down and is used to decide whether the dismissible banner gets displayed or not. (configs get grabbed at the SubnetLinodeRow level)
|
Should the warning icon and dismissible banner show up for all linode statuses, including when the linode is inactive, or just when it's booting/online? My linode was originally offline so that I could set up the unrecommended configs, and then when I navigated back its vpc, the icon/banner didn't show up. After powering on the Linode the banner and icon show up then (*after reloading, see below) When I switch the linode from a good config profile one to an unrecommended one, I need to reload the page in order to get the icon and banner to show up (when the linode is booting/active. If the linode's inactive, then the banner/icon don't show up after a page reload) -- see video: Screen going white/loading is me reloading the page. (also deleting this linode now) hmm.mov |
Confirmed with UX last Friday that we don't want these indicators displayed when the linode is inactive 👍
What was happening here is: the linode is off in order to change the config --> config is changed, query gets refetched and |
There was a problem hiding this comment.
It seems a bit odd to me for the warning text to be separate (i.e. in a notice) from the warning icon. What about removing the notice and adding the text as a tooltip using the TooltipIcon component?
https://design.linode.com/?path=/docs/components-tooltip-tooltip-icon--documentation
There was a problem hiding this comment.
autoformatting from the linter
…rning icon at the SubnetLinodeRow level
|
thanks for confirming @dwiley-akamai! Still getting the warning indicator to show up for inactive linodes. This happens if the linode had the warning indicator while online -- it stays after the linode is shut down. Might need to invalidate config queries as the linode is shutting down as well then? |
There was a problem hiding this comment.
✅ SubnetLinodeRow.test.tsx tests pass
✅ Warning icon + tooltip show up as expected when Linode is online - see comment above for offline linode
Regarding the warning icon status, how does it work for linodes with multiple configurations? Is it a similar situation to the Reboot Needed status, where it might get super confusing for linodes w multiple configs? 👀
…ts for hasUnrecommendedConfiguration()
It's a similar situation unfortunately: I created a linode with an unrecommended config profile and a recommended config profile, assigning the former to one subnet and the latter to another subnet. I booted it with the unrecommended config profile and the icon displays for both of the linode's appearances in the table:
|
|
@dwiley-akamai thanks for confirming! Is UX good with this behavior? Otherwise maybe you could pass the subnet ID into the |
What did you have in mind with the subnet ID? I'm not sure just the subnet ID is enough on its own. I wrote this out and the logic seems to work: Revised hasUnrecommendedConfiguration() and new helper functionexport const hasUnrecommendedConfiguration = (
configs: Config[],
subnet: Subnet | undefined
) => {
const subnetAssignedLinodeData = subnet?.linodes;
for (const config of configs) {
const configInterfaces = config.interfaces;
/*
If there is a VPC interface marked as active but not primary, we want to display a
message re: it not being a recommended configuration.
Rationale: when the VPC interface is not the primary interface, it can communicate
to other VMs within the same subnet, but not to VMs in a different subnet
within the same VPC.
*/
return configInterfaces.some(
(_interface) =>
_interface.active &&
_interface.purpose === 'vpc' &&
!_interface.primary &&
interfaceInSubnet(_interface.id, subnetAssignedLinodeData)
);
}
return false;
};
export const interfaceInSubnet = (
interfaceId: number,
subnetAssignedLinodeData: SubnetAssignedLinodeData[] | undefined
) => {
return subnetAssignedLinodeData?.some((subnetAssignedLinodeDatum) => {
const assignedInterfaces = subnetAssignedLinodeDatum.interfaces;
return assignedInterfaces.some(
(assignedInterface) =>
assignedInterface.id === interfaceId && assignedInterface.active
);
});
};In the scenario I described in my previous comment, this logic has the warning displayed in the subnet for the linode whose VPC interface has the unrecommended config, and not in the other subnet (same linode but whose VPC interface has the recommended config). |
|
@dwiley-akamai I wasn't clear, apologies. I meant pass in the subnet_id **in addition to the configs you were already passing in, which would allow you do do what you wrote above ^, to check if an interface was associated with the subnet_id you passed in. Glad to hear that works ^! |
|
@dwiley-akamai Wednesday before Thanksgiving shenanigans 😅 - I just looked over what you wrote above again, and realized you had a different approach. I had more in mind that you could compare the Will be re-reviewing this shortly! |
|
@coliu-akamai Thanks for the thorough testing and review -- with the most recent changes I think the expected indicators should be displayed |
There was a problem hiding this comment.
Tested ✅
- 1 config linode unrecommended config
- 1 config linode recommended config
- 2 configs linode ✅
- first one in use and unrecommended, second one recommended but not in use (warning indicator for first)
- first one not in use and unrecommended, second one in use and recommended (no warning indicators)
- first one in use and recommended, second one unrecommended but not in use (no warning indicators)
- first one not in use and recommended, second on in use and unrecommended (warning indicator for second)
- both unrecommended and first one in use (warning indicator for first)
- both unrecommended and second one in use (warning indicator for second)
- both recommended, first one in use (no warning indicator)
- both recommended, second one in use (no warning indicator)
The indicators worked as expected -- thanks Dajahi! 🚀
|
Ah - one small thing I was thinking about in terms of accessibility for the warning indicator: is there a way to use the keyboard to click on the |
Between #9916 and how we configure things for the user via the initial assignment payloads, I think we should be covered (users would have to go into the Add/Edit Config dialog to change to unrecommended configurations, so they would see the warnings).
This seems to be a limitation of MUI's |
hana-akamai
added
Approved
|
Created M3-7486 for investigating the accessibility question |
Description 📝
Assigning a linode to a VPC but not having the VPC interface be
primaryresults in situations where network connectivity issues can arise (see ticket for more details). This PR adds warning indicators on the VPC Detail page when such configurations exist: a warning icon at theSubnetLinodeRowlevelChanges 🔄
hasUnrecommendedConfigurationutility functionPreview 📷
Warning icon
How to test 🧪
Prerequisites
Verification steps
With the prereqs described above, observe:
If you go to the linode with an unrecommended configuration and adjust it by making the VPC interface (eth1) the primary interface, returning to the VPC Detail page should show that the warning icon is no longer present for that linode.
As an Author I have considered 🤔