feat: [M3-6971] Add dynamic pricing model to Volume creation flows #9569
Conversation
| const price = getClampedPrice(value, currentSize); | ||
|
|
||
| return ( | ||
| <Box marginTop={4}> | ||
| <DisplayPrice interval="mo" price={price} /> | ||
| <DisplayPrice interval="mo" price={Number(price)} /> |
There was a problem hiding this comment.
There's inconsistencies in the way price is passed to children resulting in some expecting a string and others a number, such as DisplayPrice & Currency. Just something to be aware when we'll get the values from the API, which is when we can perhaps try to consolidate
| left: `calc(${SIZE_FIELD_WIDTH}px + ${theme.spacing(2)})`, | ||
| position: 'absolute', | ||
| top: 50, | ||
| }, |
There was a problem hiding this comment.
Those additional styles prevent an existing display issue with the combination of the right helper text and the inline error.
| Before | After |
|---|---|
 |
 |
| export default compose(connected, withProfile)(VolumeDrawer); | ||
| const VolumeDrawerWithHOCs = withProfile(connected(VolumeDrawer)); | ||
|
|
||
| export default VolumeDrawerWithHOCs; |
There was a problem hiding this comment.
Refactored this to be a functional component in order to use hooks. There is much more refactor needed for volumes, a lot of those files have not been touched in a long time. However I wanted to keep the diff related to this effort and not introduce other errors not pertaining to the scope of the ticket's requirements
bnussman-akamai
added
Ready for Review
Missing Changeset
and removed
Missing Changeset
labels
abailly-akamai
changed the title
| * @param size The size of a Volume in GBs | ||
| * @returns the base price of a volume based on its size | ||
| */ | ||
| export const getDynamicVolumePrice = ({ flags, regionId, size }: Props) => { |
There was a problem hiding this comment.
Thoughts on Options instead of Props? Props make me think React Component
There was a problem hiding this comment.
Sure why not
bnussman-akamai
added
Add'tl Approval Needed
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@linode/manager": Changed | |||
There was a problem hiding this comment.
| "@linode/manager": Changed | |
| "@linode/manager": Upcoming Features |
| <span> | ||
| A single Volume can range from 10 to {MAX_VOLUME_SIZE} GB | ||
| in size and costs $0.10/GB per month. <br /> | ||
| Up to eight volumes can be attached to a single Linode. |
There was a problem hiding this comment.
| Up to eight volumes can be attached to a single Linode. | |
| Up to eight volumes can be attached to a single Linode |
We've got a double period here:
| A single Volume can range from 10 to 10240 GB in size. Up | ||
| to eight Volumes can be attached to a single Linode. | ||
| Select a Region to see cost per GB |
There was a problem hiding this comment.
This is a nitpick, but with the addition of another sentence, the way that we're displaying our copy is a little strange now, with that line break separating "Linode" onto the next line instead of at the end of the sentence. Can we potentially adjust max-width to something like 700 here to make our helper text more scannable?
bnussman-akamai
added
Approved
abailly-akamai
force-pushed
the
feat-M3-6971
branch
from
bcfd7a3 to
c7057fb
Compare
Description 📝
This PR adds dynamic (DC based) pricing to the Volume creation flows.
Preview 📷
How to test 🧪
DC-Specific Pricingfeature flag in the dev settingsFlows: