-
Notifications
You must be signed in to change notification settings - Fork 42
feat: export Roles page as a component and consume it in apps/admin #1349
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
base: main
Are you sure you want to change the base?
Changes from all commits
43ad8f6
ec4b710
5516e88
07270ae
8840d01
1e8cd80
15b15a3
e84c499
05073d2
40cf496
5331b3b
8998b08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { RolesView } from "@raystack/frontier/admin"; | ||
| import { useParams, useNavigate } from "react-router-dom"; | ||
|
|
||
| export function RolesPage() { | ||
| const { roleId } = useParams(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| return ( | ||
| <RolesView | ||
| selectedRoleId={roleId} | ||
| onSelectRole={(id) => navigate(`/roles/${encodeURIComponent(id)}`)} | ||
| onCloseDetail={() => navigate("/roles")} | ||
| /> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import type { CSSProperties, PropsWithChildren } from "react"; | ||
| import { Flex, Text } from "@raystack/apsara"; | ||
| import styles from "./page-header.module.css"; | ||
|
|
||
| export type PageHeaderTypes = { | ||
| title: string; | ||
| breadcrumb: { name: string; href?: string }[]; | ||
| className?: string; | ||
| style?: CSSProperties; | ||
| }; | ||
|
|
||
| export function PageHeader({ | ||
| title, | ||
| breadcrumb, | ||
| children, | ||
| className, | ||
| style = {}, | ||
| ...props | ||
| }: PropsWithChildren<PageHeaderTypes>) { | ||
| return ( | ||
| <Flex | ||
| align="center" | ||
| justify="between" | ||
| className={className} | ||
| style={{ padding: "16px 24px", ...style }} | ||
|
Member
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. can we move add class for this
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| {...props} | ||
| > | ||
| <Flex align="center" gap="medium"> | ||
| <Flex align="center" gap="small" className={styles.breadcrumb}> | ||
| <Text style={{ fontSize: "14px", fontWeight: "500" }}>{title}</Text> | ||
|
Member
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. fontWeight and fontSize can be props
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| {breadcrumb.map((item) => ( | ||
| <span key={item.name} className={styles.breadcrumbItem}> | ||
| {item.name} | ||
| </span> | ||
| ))} | ||
| </Flex> | ||
| </Flex> | ||
| <Flex gap="small">{children}</Flex> | ||
| </Flex> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { useEffect } from "react"; | ||
|
|
||
| interface PageTitleProps { | ||
| title?: string; | ||
| appName?: string; | ||
| } | ||
|
|
||
| export function PageTitle({ title, appName }: PageTitleProps) { | ||
| const fullTitle = title && appName ? `${title} | ${appName}` : title ?? appName ?? ""; | ||
|
|
||
| useEffect(() => { | ||
| if (fullTitle) document.title = fullTitle; | ||
| return () => { | ||
| document.title = appName ?? ""; | ||
| }; | ||
| }, [fullTitle, appName]); | ||
|
|
||
| return null; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import type { CSSProperties } from "react"; | ||
| import { Cross1Icon } from "@radix-ui/react-icons"; | ||
| import { Flex, Text } from "@raystack/apsara"; | ||
|
|
||
| type SheetHeaderProps = { | ||
| title: string; | ||
| onClick: () => void; | ||
| "data-testid"?: string; | ||
| }; | ||
|
|
||
| export function SheetHeader({ title, onClick, "data-testid": dataTestId }: SheetHeaderProps) { | ||
| return ( | ||
| <Flex justify="between" style={styles.header}> | ||
| <Text size={4} style={{ fontWeight: "500" }}> | ||
|
Member
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. fontWeight can be props
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| {title} | ||
| </Text> | ||
| <Cross1Icon | ||
| onClick={onClick} | ||
| style={{ cursor: "pointer" }} | ||
| data-testid={dataTestId ?? "admin-close-btn"} | ||
| /> | ||
| </Flex> | ||
| ); | ||
| } | ||
|
|
||
| const styles: { header: CSSProperties } = { | ||
| header: { | ||
| padding: "18px 32px", | ||
| borderBottom: "1px solid var(--rs-color-border-base-primary)", | ||
| }, | ||
| }; | ||
|
Comment on lines
+11
to
+31
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. 🧩 Analysis chain🏁 Script executed: # First, let's locate and examine the SheetHeader.tsx file
find . -name "SheetHeader.tsx" -type f | head -20Repository: raystack/frontier Length of output: 103 🏁 Script executed: # Search for Cross1Icon component definition
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "Cross1Icon" | head -10Repository: raystack/frontier Length of output: 408 🏁 Script executed: # Look for any accessibility-related attributes or patterns in the file
rg "role|aria-|keyboard" web/lib/admin/components/SheetHeader.tsx -A 3 -B 3Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Read the SheetHeader.tsx file
cat -n web/lib/admin/components/SheetHeader.tsxRepository: raystack/frontier Length of output: 1078 🏁 Script executed: # Search for Cross1Icon import and definition
rg "import.*Cross1Icon|from.*@radix" web/lib/admin/components/SheetHeader.tsx -A 2 -B 2Repository: raystack/frontier Length of output: 204 🏁 Script executed: # Look for Cross1Icon in the entire codebase to understand its source
rg "Cross1Icon" --type tsx --type ts -B 2 | head -50Repository: raystack/frontier Length of output: 88 🏁 Script executed: # Search for other uses of Cross1Icon or similar icon patterns in the codebase
rg "Cross1Icon|Icon.*onClick" -A 3 -B 1 | head -60Repository: raystack/frontier Length of output: 4304 🏁 Script executed: # Look for button patterns with icons in the codebase
rg "<button.*aria-label|type=\"button\".*Icon" -A 2 -B 2 | head -80Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Check if there are accessibility patterns or components for icon buttons
rg "IconButton|close.*button|aria-label" web/lib/admin/components -A 2 -B 2Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Search for IconButton component definition and usage
rg "IconButton" web/ -A 5 -B 2 | head -100Repository: raystack/frontier Length of output: 7568 🏁 Script executed: # Check if `@raystack/apsara` has IconButton documentation or types
find . -name "*.tsx" -o -name "*.ts" | xargs grep -l "export.*IconButton\|from.*apsara.*IconButton" | head -10Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Look for tests related to SheetHeader
find . -name "*SheetHeader*test*" -o -name "*SheetHeader*.spec.*"Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Search for any related accessibility tests or patterns
rg "admin-close-btn|testid.*close" -A 3 -B 3 | head -80Repository: raystack/frontier Length of output: 988 🏁 Script executed: # Check the imports available in SheetHeader.tsx and if IconButton can be imported
rg "from.*@raystack/apsara" web/lib/admin/components/SheetHeader.tsxRepository: raystack/frontier Length of output: 107 🏁 Script executed: # Look at how IconButton is imported in other files to understand the pattern
rg "import.*IconButton.*@raystack/apsara" web/apps/admin/src/ -B 2 | head -30Repository: raystack/frontier Length of output: 816 🏁 Script executed: # Check if SheetHeader component is used in tests and what accessibility expectations exist
find . -path "*/test*" -o -path "*/spec*" | xargs grep -l "SheetHeader\|admin-close-btn" 2>/dev/nullRepository: raystack/frontier Length of output: 45 🏁 Script executed: # Check if IconButton can be imported and used in SheetHeader
grep -r "export.*IconButton" web/lib/admin/ 2>/dev/null | head -20Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Look at an actual IconButton usage to understand required props
rg "IconButton" web/apps/admin/src/pages/invoices/list/navbar.tsx -A 5 -B 2 | head -40Repository: raystack/frontier Length of output: 638 🏁 Script executed: # Check if there's a pattern of using native button vs IconButton in headers/close actions
rg "<button.*close|<IconButton.*close" -A 3 -B 3 | head -60Repository: raystack/frontier Length of output: 43 🏁 Script executed: # Verify `@raystack/apsara` exports IconButton from web/lib
find web/lib -name "*.ts" -o -name "*.tsx" | xargs grep "export.*IconButton" 2>/dev/nullRepository: raystack/frontier Length of output: 45 Use IconButton for the close action to meet accessibility requirements. ♿ Proposed fix export function SheetHeader({ title, onClick, "data-testid": dataTestId }: SheetHeaderProps) {
return (
<Flex justify="between" style={styles.header}>
<Text size={4} style={{ fontWeight: "500" }}>
{title}
</Text>
- <Cross1Icon
+ <IconButton
+ aria-label="Close"
onClick={onClick}
- style={{ cursor: "pointer" }}
data-testid={dataTestId ?? "admin-close-btn"}
- />
+ >
+ <Cross1Icon />
+ </IconButton>
</Flex>
);
}Also update the import to include IconButton: -import { Flex, Text } from "@raystack/apsara";
+import { Flex, Text, IconButton } from "@raystack/apsara";🧰 Tools🪛 GitHub Check: JS SDK Lint[warning] 17-17: 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| .breadcrumb { | ||
| list-style: none; | ||
| padding: 0; | ||
| margin: 0; | ||
| display: flex; | ||
| font-size: 16px; | ||
|
Member
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. can we use css variables here
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| } | ||
|
|
||
| .breadcrumb a { | ||
| text-decoration: none; | ||
| color: #007bff; | ||
|
Member
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. can we use css variables here
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| display: inline-block; | ||
| margin-right: 5px; | ||
| } | ||
|
|
||
| .breadcrumb a::before { | ||
| content: ">"; | ||
| color: #666; | ||
|
Member
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. same as above
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| } | ||
|
|
||
| .breadcrumb a:first-child::before { | ||
| content: ""; | ||
| } | ||
|
|
||
| .breadcrumb a:last-child { | ||
| margin-right: 0; | ||
| } | ||
|
|
||
| .breadcrumbItem { | ||
| margin-right: 5px; | ||
| } | ||
|
|
||
| .breadcrumbItem::before { | ||
| content: ">"; | ||
| color: #666; | ||
|
Member
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. same as above
Contributor
Author
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. Out of scope for this PR. Will be picked up in another ticket. |
||
| margin-right: 5px; | ||
| } | ||
|
|
||
| .breadcrumbItem:first-of-type::before { | ||
| content: ""; | ||
| margin-right: 0; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| "use client"; | ||
|
|
||
| export { default as RolesView } from "./views/roles"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { timestampDate, type Timestamp } from "@bufbuild/protobuf/wkt"; | ||
|
|
||
| export function timestampToDate(timestamp?: Timestamp): Date | null { | ||
| if (!timestamp) return null; | ||
| return timestampDate(timestamp); | ||
| } | ||
|
|
||
| export type TimeStamp = Timestamp; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| export function reduceByKey<T extends Record<string, unknown>>( | ||
| data: T[], | ||
| key: keyof T | ||
| ): Record<string, T> { | ||
| return data.reduce( | ||
| (acc, value) => ({ | ||
| ...acc, | ||
| [String(value[key])]: value, | ||
| }), | ||
| {} as Record<string, T> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,25 +1,43 @@ | ||||||||||||||||||||||
| import { EmptyState, Flex, DataTable, Sheet } from "@raystack/apsara"; | ||||||||||||||||||||||
| import { | ||||||||||||||||||||||
| Outlet, | ||||||||||||||||||||||
| useNavigate, | ||||||||||||||||||||||
| useOutletContext, | ||||||||||||||||||||||
| useParams, | ||||||||||||||||||||||
| } from "react-router-dom"; | ||||||||||||||||||||||
| import { useCallback, useState } from "react"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { reduceByKey } from "~/utils/helper"; | ||||||||||||||||||||||
| import { reduceByKey } from "../../utils/helper"; | ||||||||||||||||||||||
| import { getColumns } from "./columns"; | ||||||||||||||||||||||
| import { RolesHeader } from "./header"; | ||||||||||||||||||||||
| import { ExclamationTriangleIcon } from "@radix-ui/react-icons"; | ||||||||||||||||||||||
| import PageTitle from "~/components/page-title"; | ||||||||||||||||||||||
| import { PageTitle } from "../../components/PageTitle"; | ||||||||||||||||||||||
| import styles from "./roles.module.css"; | ||||||||||||||||||||||
| import { SheetHeader } from "~/components/sheet/header"; | ||||||||||||||||||||||
| import { SheetHeader } from "../../components/SheetHeader"; | ||||||||||||||||||||||
| import { FrontierServiceQueries, Role } from "@raystack/proton/frontier"; | ||||||||||||||||||||||
| import { useQuery } from "@connectrpc/connect-query"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| type ContextType = { role: Role | null }; | ||||||||||||||||||||||
| export default function RoleList() { | ||||||||||||||||||||||
| const { roleId } = useParams(); | ||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||
| import RoleDetails from "./details"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export type RolesViewProps = { | ||||||||||||||||||||||
| selectedRoleId?: string; | ||||||||||||||||||||||
| onSelectRole?: (roleId: string) => void; | ||||||||||||||||||||||
| onCloseDetail?: () => void; | ||||||||||||||||||||||
| appName?: string; | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export default function RolesView({ | ||||||||||||||||||||||
| selectedRoleId: controlledRoleId, | ||||||||||||||||||||||
| onSelectRole, | ||||||||||||||||||||||
| onCloseDetail, | ||||||||||||||||||||||
| appName, | ||||||||||||||||||||||
| }: RolesViewProps = {}) { | ||||||||||||||||||||||
| const [internalRoleId, setInternalRoleId] = useState<string | undefined>(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const selectedRoleId = controlledRoleId ?? internalRoleId; | ||||||||||||||||||||||
| const handleClose = useCallback( | ||||||||||||||||||||||
| () => (onCloseDetail ? onCloseDetail() : setInternalRoleId(undefined)), | ||||||||||||||||||||||
| [onCloseDetail] | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| const handleRowClick = useCallback( | ||||||||||||||||||||||
| (role: Role) => | ||||||||||||||||||||||
| onSelectRole ? onSelectRole(role.id ?? "") : setInternalRoleId(role.id ?? undefined), | ||||||||||||||||||||||
| [onSelectRole] | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
Comment on lines
+36
to
+40
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. Inconsistent handling of undefined role ID. Line 38 converts 🐛 Proposed fix for consistency const handleRowClick = useCallback(
(role: Role) =>
- onSelectRole ? onSelectRole(role.id ?? "") : setInternalRoleId(role.id ?? undefined),
+ onSelectRole ? onSelectRole(role.id ?? "") : setInternalRoleId(role.id),
[onSelectRole]
);Note: 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const { | ||||||||||||||||||||||
| data: roles = [], | ||||||||||||||||||||||
|
|
@@ -35,12 +53,6 @@ | |||||||||||||||||||||
| ); | ||||||||||||||||||||||
| const roleMapByName = reduceByKey(roles ?? [], "id"); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function onClose() { | ||||||||||||||||||||||
| navigate("/roles"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| function onRowClick(role: Role) { | ||||||||||||||||||||||
| navigate(`${encodeURIComponent(role.id ?? "")}`); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (isError) { | ||||||||||||||||||||||
| console.error("ConnectRPC Error:", error); | ||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
|
|
@@ -58,14 +70,14 @@ | |||||||||||||||||||||
| const columns = getColumns(); | ||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| <DataTable | ||||||||||||||||||||||
| onRowClick={onRowClick} | ||||||||||||||||||||||
| onRowClick={handleRowClick} | ||||||||||||||||||||||
| data={roles} | ||||||||||||||||||||||
| columns={columns} | ||||||||||||||||||||||
| mode="client" | ||||||||||||||||||||||
| defaultSort={{ name: "title", order: "asc" }} | ||||||||||||||||||||||
| isLoading={isLoading}> | ||||||||||||||||||||||
| <Flex direction="column"> | ||||||||||||||||||||||
| <PageTitle title="Roles" /> | ||||||||||||||||||||||
| <PageTitle title="Roles" appName={appName} /> | ||||||||||||||||||||||
| <RolesHeader /> | ||||||||||||||||||||||
| <DataTable.Content | ||||||||||||||||||||||
| emptyState={noDataChildren} | ||||||||||||||||||||||
|
|
@@ -74,18 +86,16 @@ | |||||||||||||||||||||
| table: styles.table, | ||||||||||||||||||||||
| }} | ||||||||||||||||||||||
| /> | ||||||||||||||||||||||
| <Sheet open={roleId !== undefined}> | ||||||||||||||||||||||
| <Sheet open={selectedRoleId !== undefined}> | ||||||||||||||||||||||
| <Sheet.Content className={styles.sheetContent}> | ||||||||||||||||||||||
| <SheetHeader | ||||||||||||||||||||||
| title="Role Details" | ||||||||||||||||||||||
| onClick={onClose} | ||||||||||||||||||||||
| data-test-id="role-details-header" | ||||||||||||||||||||||
| onClick={handleClose} | ||||||||||||||||||||||
| data-testid="role-details-header" | ||||||||||||||||||||||
| /> | ||||||||||||||||||||||
| <Flex className={styles.sheetContentBody}> | ||||||||||||||||||||||
| <Outlet | ||||||||||||||||||||||
| context={{ | ||||||||||||||||||||||
| role: roleId ? roleMapByName[roleId] : null, | ||||||||||||||||||||||
| }} | ||||||||||||||||||||||
| <RoleDetails | ||||||||||||||||||||||
| role={selectedRoleId ? roleMapByName[selectedRoleId] ?? null : null} | ||||||||||||||||||||||
| /> | ||||||||||||||||||||||
| </Flex> | ||||||||||||||||||||||
| </Sheet.Content> | ||||||||||||||||||||||
|
|
@@ -95,10 +105,6 @@ | |||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export function useRole() { | ||||||||||||||||||||||
| return useOutletContext<ContextType>(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export const noDataChildren = ( | ||||||||||||||||||||||
| <EmptyState icon={<ExclamationTriangleIcon />} heading="0 role created" /> | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
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.
Just name it RolesPage
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.
Done