Skip to content
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

fix: resolve empty value display in DoubleTag and update BAIIntervalView #2813

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Nov 5, 2024

Changes:

  • Replaced BAIIntervalText with new BAIIntervalView component that supports render props
  • Updated DoubleTag component to handle string highlighting and simplified value types
  • Refactored tag rendering to use object-based values consistently
  • Added type safety improvements for tag labels and values

Rationale:
Improves component reusability and type safety by introducing a more flexible interval view component with render prop support. Standardizes the way tags are rendered across the application while adding proper highlighting capabilities.

How to test:

  1. Check agent list running time displays correctly
  2. Verify session elapsed time shows properly
  3. Test customized image tags display correctly

Example usage:

<BAIIntervalView
  callback={() => baiClient.utils.elapsedTime(value, Date.now())}
  delay={1000}
  render={(intervalValue) => (
    <DoubleTag
      values={[
        { label: 'Running' },
        { label: intervalValue }
      ]}
    />
  )}
/>

Checklist:

  • Test interval updates across all usage locations
  • Verify tag highlighting works with search functionality
  • Check type safety of tag value props
  • Ensure backwards compatibility with existing tag implementations

Screenshots:
Before
image.png

After
image.png

Copy link

graphite-app bot commented Nov 5, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the size:S 10~30 LoC label Nov 5, 2024
@agatha197 agatha197 requested a review from yomybaby November 5, 2024 05:00
Copy link

github-actions bot commented Nov 5, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
5.02% (-0.02% 🔻)
367/7318
🔴 Branches
4.4% (+0.01% 🔼)
221/5027
🔴 Functions
2.94% (-0.01% 🔻)
71/2414
🔴 Lines
4.92% (-0.02% 🔻)
352/7157
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / BAIIntervalView.tsx
0% 0% 0% 0%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴 hooks/index.tsx
37.96% (-4.67% 🔻)
24.39% (-3.39% 🔻)
19.51% (-1.54% 🔻)
37.31% (-4.7% 🔻)
🔴 helper/index.tsx
50.97% (-1.35% 🔻)
47.22%
39.47% (-1.07% 🔻)
52.24% (-1.61% 🔻)

Test suite run success

109 tests passing in 14 suites.

Report generated by 🧪jest coverage report action from 9c66cb1

@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 498cfd6 to 622b3db Compare November 6, 2024 04:48
@github-actions github-actions bot added size:XS ~10 LoC and removed size:S 10~30 LoC labels Nov 6, 2024
react/src/components/DoubleTag.tsx Outdated Show resolved Hide resolved
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 622b3db to 0d11bdd Compare November 8, 2024 02:42
@github-actions github-actions bot added size:S 10~30 LoC and removed size:XS ~10 LoC labels Nov 8, 2024
@agatha197 agatha197 marked this pull request as draft November 8, 2024 03:13
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 0d11bdd to f42a71f Compare November 8, 2024 03:36
@github-actions github-actions bot added size:L 100~500 LoC and removed size:S 10~30 LoC labels Nov 8, 2024
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from f42a71f to 2c6d6e1 Compare November 8, 2024 03:38
@agatha197 agatha197 changed the title fix: remove tag when DoubleTag's label has empty children refactor: replace BAIIntervalText with BAIIntervalView and add render prop Nov 8, 2024
@agatha197 agatha197 changed the title refactor: replace BAIIntervalText with BAIIntervalView and add render prop fix: resolve empty value display in DoubleTag and update BAIIntervalView Nov 8, 2024
@agatha197 agatha197 marked this pull request as ready for review November 8, 2024 03:51
@agatha197 agatha197 requested a review from yomybaby November 8, 2024 03:51
react/src/components/AgentList.tsx Outdated Show resolved Hide resolved
react/src/components/DoubleTag.tsx Outdated Show resolved Hide resolved
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 2c6d6e1 to 711bb98 Compare November 8, 2024 05:07
@agatha197 agatha197 requested a review from yomybaby November 8, 2024 05:08
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 711bb98 to 9c66cb1 Compare November 12, 2024 07:57
@agatha197 agatha197 changed the base branch from main to feature/make-image-lists-resizable November 12, 2024 07:57
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 9c66cb1 to 3ccf2e1 Compare November 12, 2024 08:01
@agatha197 agatha197 force-pushed the feature/make-image-lists-resizable branch from 8c89ece to 4b7ae74 Compare November 12, 2024 08:24
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 3ccf2e1 to 7c251ec Compare November 12, 2024 08:25
@agatha197 agatha197 force-pushed the feature/make-image-lists-resizable branch from 4b7ae74 to 0a90887 Compare November 12, 2024 08:32
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 7c251ec to 509c379 Compare November 12, 2024 08:32
@agatha197 agatha197 force-pushed the feature/make-image-lists-resizable branch from 0a90887 to 64a56cd Compare November 12, 2024 08:36
@agatha197 agatha197 force-pushed the fix/remove-tag-when-label-has-empty-children branch from 509c379 to 98ef302 Compare November 12, 2024 08:36
@yomybaby yomybaby force-pushed the feature/make-image-lists-resizable branch from 64a56cd to 482c0ac Compare November 12, 2024 08:45
@yomybaby yomybaby force-pushed the fix/remove-tag-when-label-has-empty-children branch from 98ef302 to f644e87 Compare November 12, 2024 08:45
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

graphite-app bot commented Nov 12, 2024

Merge activity

…alView` (#2813)

**Changes:**
- Replaced `BAIIntervalText` with new `BAIIntervalView` component that supports render props
- Updated `DoubleTag` component to handle string highlighting and simplified value types
- Refactored tag rendering to use object-based values consistently
- Added type safety improvements for tag labels and values

**Rationale:**
Improves component reusability and type safety by introducing a more flexible interval view component with render prop support. Standardizes the way tags are rendered across the application while adding proper highlighting capabilities.

**How to test:**
1. Check agent list running time displays correctly
2. Verify session elapsed time shows properly
3. Test customized image tags display correctly

Example usage:
```tsx
<BAIIntervalView
  callback={() => baiClient.utils.elapsedTime(value, Date.now())}
  delay={1000}
  render={(intervalValue) => (
    <DoubleTag
      values={[
        { label: 'Running' },
        { label: intervalValue }
      ]}
    />
  )}
/>
```

**Checklist:**
- [ ] Test interval updates across all usage locations
- [ ] Verify tag highlighting works with search functionality
- [ ] Check type safety of tag value props
- [ ] Ensure backwards compatibility with existing tag implementations

**Screenshots:**
Before
![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/497082c2-e79a-43bd-a86a-70554e2984c3.png)

After
![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/9b654ec0-cfa6-400d-88d4-bd1cebcd99a2.png)
@yomybaby yomybaby force-pushed the feature/make-image-lists-resizable branch from 482c0ac to 506a1a1 Compare November 12, 2024 08:56
@yomybaby yomybaby force-pushed the fix/remove-tag-when-label-has-empty-children branch from f644e87 to 7bb0c92 Compare November 12, 2024 08:57
Base automatically changed from feature/make-image-lists-resizable to main November 12, 2024 08:59
@graphite-app graphite-app bot merged commit 7bb0c92 into main Nov 12, 2024
4 checks passed
@graphite-app graphite-app bot deleted the fix/remove-tag-when-label-has-empty-children branch November 12, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants