-
Notifications
You must be signed in to change notification settings - Fork 370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upcoming: [M3-8340] - Update Manage Image Regions Drawer based on UX feedback #10674
Changes from 4 commits
d4c3e13
80efb4a
4822e24
2abbd49
6ce79c1
6df361f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@linode/manager": Upcoming Features | ||
--- | ||
|
||
Update Manage Image Regions Drawer based on UX feedback ([#10674](https://github.com/linode/manager/pull/10674)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
import { Region } from '@linode/api-v4'; | ||
import CloseIcon from '@mui/icons-material/Close'; | ||
import React from 'react'; | ||
|
||
|
@@ -24,6 +23,7 @@ import type { | |
DisableRegionOption, | ||
RegionMultiSelectProps, | ||
} from './RegionSelect.types'; | ||
import type { Region } from '@linode/api-v4'; | ||
|
||
interface LabelComponentProps { | ||
region: Region; | ||
|
@@ -67,7 +67,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => { | |
selectedIds, | ||
sortRegionOptions, | ||
width, | ||
onClose, | ||
...rest | ||
} = props; | ||
|
||
const { | ||
|
@@ -115,7 +115,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => { | |
return getRegionCountryGroup(option); | ||
}} | ||
onChange={(_, selectedOptions) => | ||
onChange(selectedOptions.map((region) => region.id)) | ||
onChange(selectedOptions?.map((region) => region.id) ?? []) | ||
} | ||
renderOption={(props, option, { selected }) => { | ||
if (!option.site_type) { | ||
|
@@ -155,7 +155,6 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => { | |
InputProps: { | ||
required, | ||
}, | ||
placeholder: selectedRegions.length > 0 ? '' : placeholder, | ||
tooltipText: helperText, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is a good default so I moved this special logic into We want the placeholder to always show in the Manage Regions Drawer and this logic was messing that up |
||
}} | ||
autoHighlight | ||
|
@@ -172,7 +171,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => { | |
options={regionOptions} | ||
placeholder={placeholder ?? 'Select Regions'} | ||
value={selectedRegions} | ||
onClose={onClose} | ||
{...rest} | ||
/> | ||
</StyledAutocompleteContainer> | ||
{selectedRegions.length > 0 && SelectedRegionsList && ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,7 +68,7 @@ export interface RegionSelectProps< | |
|
||
export interface RegionMultiSelectProps | ||
extends Omit< | ||
EnhancedAutocompleteProps<Region, false>, | ||
EnhancedAutocompleteProps<Region, true>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
'label' | 'onChange' | 'options' | ||
> { | ||
SelectedRegionsList?: React.ComponentType<{ | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -29,13 +29,15 @@ export const AccessKeyRegions = (props: Props) => { | |||||
|
||||||
return ( | ||||||
<RegionMultiSelect | ||||||
onChange={onChange} | ||||||
placeholder={ | ||||||
selectedRegion.length > 0 ? ' ' : 'Select regions or type to search' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for this space?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I leave it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, fixed with 6df361f |
||||||
} | ||||||
currentCapability="Object Storage" | ||||||
disabled={disabled} | ||||||
errorText={errorText} | ||||||
isClearable={false} | ||||||
label="Regions" | ||||||
placeholder="Select Regions or type to search" | ||||||
onChange={onChange} | ||||||
regions={regions ?? []} | ||||||
required={required} | ||||||
selectedIds={selectedRegion} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows us to override any prop of the Autocomplete.
Needed to make this change so I could override
renderTags