- Notifications
You must be signed in to change notification settings - Fork929
chore: update workspaces page filter to include organization controls#14597
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| <> | ||
| <SearchFieldSkeleton /> | ||
| <MenuSkeleton /> |
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.
We forgot to put a fourthMenuSkeleton here for the orgs dropdown
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.
it needs to be conditional onshowOrganizations :^)
ParkreinerSep 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Whoops
| @@ -180,101 +175,3 @@ const ResourceTypeMenu: FC<ResourceTypeMenuProps> = ({ menu, width }) => { | |||
| /> | |||
| ); | |||
| }; | |||
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.
All moved to the orgs/filtering file
| <> | ||
| <BaseSkeleton width="100%" /> | ||
| {optionsSkeleton} | ||
| </> |
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.
Inlined the base skeleton because:
- I don't think there will ever be a situation where we don't want a skeleton for the main search box itself
- It felt weird that the component was never in control ofany of its skeletons
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.
wonderful 😄
| @@ -1,6 +1,6 @@ | |||
| import { API } from "api/api"; | |||
| import type { AuditLogResponse } from "api/typesGenerated"; | |||
| import { useFilterParamsKey } from "components/Filter/filter"; | |||
| import { useFilterParamsKey } from "components/Filter/Filter"; | |||
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.
MY HERO
| @@ -52,7 +52,7 @@ export const SelectFilter: FC<SelectFilterProps> = ({ | |||
| <SelectMenuTrigger> | |||
| <SelectMenuButton | |||
| startIcon={selectedOption?.startIcon} | |||
| css={{ width, flexGrow: 1 }} | |||
| css={{flexBasis:width, flexGrow: 1 }} | |||
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.
flexBasis!!!!! you're literally so good at your job
| const addMeAsFirstOption = (options: SelectFilterOption[]) => { | ||
| options= options.filter((option) =>option.value !== me.username); | ||
| const addMeAsFirstOption = (options:readonlySelectFilterOption[]) => { | ||
| const filtered= options.filter((o) =>o.value !== me.username); |
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.
the only way this line would make me happier is if you did the thing I like where the closure names the parameterit
| // Have to specify min width so that, as we keep adding more and | ||
| // control options to the filter row, the text box doesn't have a | ||
| // risk of shrinking so much that it becomes un-clickable | ||
| css={{ minWidth: "320px" }} |
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.
| // Have to specify min width so that, as we keep adding more and | |
| // control options to the filter row, the text box doesn't have a | |
| // risk of shrinking so much that it becomes un-clickable | |
| css={{minWidth:"320px"}} | |
| // We specify `minWidth` so that the text box can't shrink so much that | |
| // it becomes un-clickable as we add more filter controls. |
| <> | ||
| <SearchFieldSkeleton /> | ||
| <MenuSkeleton /> |
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.
it needs to be conditional onshowOrganizations :^)
| menu: StatusFilterMenu; | ||
| }>; | ||
| export const StatusMenu: FC<StatusMenuProps> = ({ width, menu }) => { |
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.
thank you 🙏
…synced for storybook
* chore: add tests for WorkspacePage cross-page navigation* fix: update story to use mock organizations menu
5aa54be intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Part 2/3 for#14435
Changes made
Screenshots
Notes