-
Notifications
You must be signed in to change notification settings - Fork 350
feat: Add ability to disable data sources #1567
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
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,8 @@ | ||
| --- | ||
| "@hyperdx/common-utils": patch | ||
| "@hyperdx/api": patch | ||
| "@hyperdx/app": patch | ||
| --- | ||
|
|
||
| Add ability to disable data sources with improved UX | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,30 +156,43 @@ export function SourcesList({ | |
|
|
||
| {sources?.map((s, index) => ( | ||
| <React.Fragment key={s.id}> | ||
| <Flex justify="space-between" align="center"> | ||
| <div> | ||
| <Text size={textSize} fw={500}> | ||
| {s.name} | ||
| </Text> | ||
| <Text size={subtextSize} c="dimmed" mt={4}> | ||
| <Group gap="xs"> | ||
| {capitalizeFirstLetter(s.kind)} | ||
| <Group gap={4}> | ||
| <IconServer size={iconSize} /> | ||
| {connections?.find(c => c.id === s.connection)?.name} | ||
| </Group> | ||
| <Group gap={4}> | ||
| {s.from && ( | ||
| <> | ||
| <IconStack size={iconSize} /> | ||
| {s.from.databaseName} | ||
| {s.kind === SourceKind.Metric ? '' : '.'} | ||
| {s.from.tableName} | ||
| </> | ||
| )} | ||
| <Flex | ||
| justify="space-between" | ||
| align="center" | ||
| opacity={s.disabled ? 0.5 : 1} | ||
| style={{ | ||
| transition: 'opacity 0.2s ease', | ||
| }} | ||
| > | ||
| <div style={{ flex: 1 }}> | ||
|
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. @elizabetdev can you provide UX feedback here? My gut tells me this feature won't be one of the main things people are doing it, so we should move it inside the form itself (see screenshots): Example of where to put disabled potentially Closed UI (toggle could be removed and add "Disabled" badge to details row?)
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. @alok87 functionality wise, things look good. pending @elizabetdev's feedback we could simplify some form code, lets see what she says :)
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.
Contributor
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. @alok87 The enabled/disabled toggle placement looks good to me 👍 Since it’s a high-level state, I agree it shouldn’t be buried under “Optional Fields.” For the disabled state in the list view, the reduced opacity works well to show the status. One small thing to watch out for is the expand or collapse arrow (chevron). It would be good to keep that at full opacity. If the arrow dims along with the text, the row can feel unclickable or read-only. Keeping the arrow bright helps signal that the row is still interactive and can be opened to update settings or re-enable it. |
||
| <Flex align="center" gap="sm"> | ||
| <div> | ||
| <Group gap="xs" align="center"> | ||
| <Text size={textSize} fw={500}> | ||
| {s.name} | ||
| </Text> | ||
| </Group> | ||
| </Group> | ||
| </Text> | ||
| <Text size={subtextSize} c="dimmed" mt={4}> | ||
| <Group gap="xs"> | ||
| {capitalizeFirstLetter(s.kind)} | ||
| <Group gap={4}> | ||
| <IconServer size={iconSize} /> | ||
| {connections?.find(c => c.id === s.connection)?.name} | ||
| </Group> | ||
| <Group gap={4}> | ||
| {s.from && ( | ||
| <> | ||
| <IconStack size={iconSize} /> | ||
| {s.from.databaseName} | ||
| {s.kind === SourceKind.Metric ? '' : '.'} | ||
| {s.from.tableName} | ||
| </> | ||
| )} | ||
| </Group> | ||
| </Group> | ||
| </Text> | ||
| </div> | ||
| </Flex> | ||
| </div> | ||
| <ActionIcon | ||
| variant="secondary" | ||
|
|
||




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.
Thoughts on removing this code and just allowing the form to manage updating this value on save?