upcoming: [M3-7357] - Add AGLB Helper Text and Copy by bnussman-akamai · Pull Request #9908 · linode/manager

bnussman-akamai · 2023-11-15T20:52:07Z

Description 📝

Adds copy and helper text for the AGLB project 📝

How to test 🧪

Prerequisites

Turn on the MSW or test with the dev-test-aglb account in dev

Verification steps

Check over all copy added throughout AGLB 👀
- Check spelling, capitalization, styling 🎨
- Things might not be 100% perfect or final, I'm doing my best to work with what we've got.

As an Author I have considered 🤔

mjac0bs

Nice job tracking all these copy edits down. 🔍

I've left a handful of nitpicks and a couple questions. The most significant of which was: in several instances (mostly Routes/Rules and Service Target drawers), I see copy updates that aren't reflected in the mocks. Is there another source of truth?

mjac0bs · 2023-11-27T20:32:58Z

...s/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationForm.tsx

          <Autocomplete
            textFieldProps={{
-              labelTooltipText: 'TODO: AGLB',
+              labelTooltipText: CONFIGURATION_COPY.Protocol,


This tooltip needs an interactive prop in order to allow the user to click on the link. With the current behavior, the tooltip popover disappears before a user can click it.

Screen.Recording.2023-11-27.at.12.30.56.PM.mov

Unfortunately, our TextField doesn't support that... Might have to do this as a follow up

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/constants.tsx

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/constants.tsx

mjac0bs · 2023-11-27T21:13:44Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.tsx

Optional nit: it's a little weird for UX for some of the drawer content to wrap differently once the session stickiness is toggled. I didn't look into the styling, but if it's an easy fix then it would be nice to change.

Screen.Recording.2023-11-27.at.12.56.48.PM.mov

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/AddRouteDrawer.tsx

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/constants.ts

mjac0bs

🧼 Thanks for the updates! Everything addressed looks good now.

As far as the textfield tooltip: yeah, I figured that was going to be an issue. A follow up PR sounds good to me; we'd have run into that issue eventually anyway.

In my second pass through, I checked all screen sizes. I'm not going to hold up approving this PR for this, but there were two small edge case issues I noticed in the Add Rule drawer on small mobile screens:

The match value text field is prone to overflowing the container
The Cookie type tooltip gets cut off in its last sentence

Screenshots

bnussman-akamai · 2023-11-27T22:28:28Z

Don't worry, I will definitely take a second PR to clean up and refine things that need to be improved/changed with Sandra's review

cpathipa

Over all looks great! Verified the copy and no spelling errors found.

cpathipa · 2023-11-28T15:56:40Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/constants.ts

@@ -0,0 +1,42 @@
+import type { Certificate } from '@linode/api-v4';
+


I like the idea of keeping tooltips copy in constants. No spelling errors found in this file.

cpathipa · 2023-11-28T16:17:08Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/constants.tsx

@@ -0,0 +1,41 @@
+import React from 'react';
+


Looks good! No spelling errors found here.
@bnussman-akamai Any thoughts in keeping this files as contants.ts and having this getConfigurationPayloadFromConfiguration function and Protocol in a util file?

cpathipa · 2023-11-28T16:36:50Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/constants.ts

+        'For TCP load balancers, a rule consists of service targets and the percentage of incoming requests that should be directed to each target. Add as many service targets as required, but the percentages for all targets must total 100%.',
+    },
+    MatchValue: {
+      header: 'The format for http header is: X-name=value.',


The format is X-name=value or X-name:value ?

X-name=value should be correct

cpathipa · 2023-11-28T16:51:07Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/constants.ts

@@ -0,0 +1,41 @@
+export const ROUTE_COPY = {


No spelling errors in this file.

cpathipa · 2023-11-28T17:20:35Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/ServiceTargets/constants.tsx

@@ -0,0 +1,144 @@
+import React from 'react';
+


Looks good. No spelling errors found in this file.

add service target initial copy

3f6379a

bnussman-akamai added the ACLB Relating to the Akamai Cloud Load Balancer label Nov 15, 2023

bnussman-akamai self-assigned this Nov 15, 2023

bnussman added 8 commits November 16, 2023 16:03

clean up files and import

cc5ff04

Merge branch 'develop' into M3-7357-add-aglb-copy

c375581

add initial configuration copy

7544a0e

add inital route copy

d92b558

more copy, slowly getting it all

83c9d34

continue adding copy

199f5df

add comment

87a93fa

Added changeset: Add AGLB copy and docs links

30dc6aa

bnussman-akamai marked this pull request as ready for review November 21, 2023 20:47

bnussman-akamai requested a review from a team as a code owner November 21, 2023 20:47

bnussman-akamai requested review from cliu-akamai and cpathipa and removed request for a team November 21, 2023 20:47

bnussman-akamai added the Ready for Review label Nov 21, 2023

bnussman-akamai requested a review from mjac0bs November 21, 2023 20:53

fix e2e test

a48c943

mjac0bs reviewed Nov 27, 2023

View reviewed changes

bnussman added 2 commits November 27, 2023 16:45

feedback

ed6264a

more feedback

6e8df96

mjac0bs approved these changes Nov 27, 2023

View reviewed changes

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Nov 27, 2023

cpathipa approved these changes Nov 28, 2023

View reviewed changes

bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Nov 28, 2023

bnussman-akamai merged commit fec8b2d into linode:develop Nov 28, 2023

mjac0bs mentioned this pull request Dec 11, 2023

Release v1.108.0 - staging → master #9986

Merged

		@@ -0,0 +1,42 @@
		import type { Certificate } from '@linode/api-v4';

Conversation

bnussman-akamai commented Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description 📝

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

Uh oh!

mjac0bs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mjac0bs left a comment

Choose a reason for hiding this comment

Uh oh!

bnussman-akamai commented Nov 27, 2023

Uh oh!

cpathipa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bnussman-akamai commented Nov 15, 2023 •

edited

Loading