azure v5: change 'None' node pool AZ selection to 'Not specified' #1941
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
axbarsan
commented
Oct 20, 2020
|
|
||
| return ( | ||
| <RUMActionTarget | ||
| name={mergeActionNames(RUMActions.SelectAZSelection, typeName)} |
There was a problem hiding this comment.
The RUM action name is now computed here, by appending the selection type to the end of the base action name (e.g. SELECT_AZ_SELECTION_AUTOMATIC)
|
i would say: "By not specifying the availability zone, Azure will select the zone by itself where the VM size is most available." |
T-Kukawka
approved these changes
Oct 21, 2020
Updated the screenshot 👍🏻 |
marians
reviewed
Oct 21, 2020
src/components/Cluster/ClusterDetail/AddNodePool/AddNodePoolAZSelection.tsx
Outdated
Show resolved
Hide resolved
marians
reviewed
Oct 21, 2020
src/components/Cluster/ClusterDetail/AddNodePool/AddNodePoolAZSelection.tsx
Outdated
Show resolved
Hide resolved
marians
reviewed
Oct 21, 2020
src/components/Cluster/ClusterDetail/AddNodePool/AddNodePoolAZSelection.tsx
Outdated
Show resolved
Hide resolved
|
LGTM! Gave some input regarding wording. |
axbarsan
deleted the
azure-v5/change-az-selection-none-to-not-specified
branch
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.
Closes https://github.com/giantswarm/giantswarm/issues/13843
In this PR, the
NoneNP AZ selection option is changed toNot specifiedand a longer explanation is now provided. Also, before, the explanation was shown in a tooltip, which was different from other options. That is now aligned, as well.Unfortunately, to be able to make the option look like the others, it was easier to rewrite the whole AZ selector, than to add the option to the old implementation. In the previous implementation, we were relying on fixed heights, and a lot of CSS class magic. It would've required a lot of hacks to convert the new option and make everything behave correctly on all providers.
In the new implementation, I'm leveraging the
Collapsecomponent fromreact-bootstrap. Since we discussed about switching to a UI library, I didn't bother writing custom code for that.The new implementation looks almost 100% the same as the old one, but the transitions are now way quicker, and there are no glitches when switching between
AutomaticandManual, anymore.Preview