Unstable experimental build for TaxEstimateView #984

darrensapalo · 2025-11-13T03:22:11Z

Description

Groundwork for the TaxEstimateView component. Internal unstable build of TaxEstimateView.

Changes

New views and components related to tax estimation.

Out of scope

API, data schema, hooks integration (See: Tax estimates - preliminary API, shape, and hooks integration #1002)

Blockers

How this has been tested?

Visual inspection with the demo-finguard app.

References

Tax estimates - preliminary API, shape, and hooks integration #1002

doneel · 2025-11-13T19:27:03Z

src/views/TaxEstimates/AdjustedGrossIncomeSection.tsx

+                        <Span
+                          size='md'
+                          className={item.isDeduction ? 'Layer__tax-estimate__deduction-item' : ''}


Layer__tax-estimate__deduction-item is just adding padding - we can do that directly on the span without the class name

I think we do need it- because spans only have padding-block but not padding-inline (left-right).

Does it make sense to extend Span to have pb but also pi?

doneel · 2025-11-13T19:35:02Z

src/views/TaxEstimates/AdjustedGrossIncomeSection.tsx

+                                <Heading size='md' className='Layer__tax-estimate__agi-total-amount'>
+                                  {convertNumberToCurrency(item.amount)}
+                                </Heading>
+                              </TooltipTrigger>
+                              <TooltipContent>
+                                <Span className='Layer__tax-estimate__tooltip' size='sm'>{item.formula}</Span>


Layer__tax-estimate__tooltip and Layer__tax-estimate__agi-total-amount are just for colors, I expect we can do that with properties on our text components.

I'm definitely okay with some shortcuts here, but this one feels like it might be even easier to use text props here.

We don't have properties on our TextStyleProps to determine colors.

src/views/TaxFiling/taxEstimate.scss

I don't want to make design decisions for our buttons under crunch time, so I'd rather this be a specific class name that we later clean up (if we do decide to extend our Span component.

darrensapalo · 2025-11-14T01:26:09Z

src/views/TaxEstimates/AdjustedGrossIncomeSection.tsx

+                        <Span
+                          size='md'
+                          className={item.isDeduction ? 'Layer__tax-estimate__deduction-item' : ''}


I think we do need it- because spans only have padding-block but not padding-inline (left-right).

Does it make sense to extend Span to have pb but also pi?

darrensapalo · 2025-11-14T01:30:34Z

src/views/TaxEstimates/AdjustedGrossIncomeSection.tsx

+                                <Heading size='md' className='Layer__tax-estimate__agi-total-amount'>
+                                  {convertNumberToCurrency(item.amount)}
+                                </Heading>
+                              </TooltipTrigger>
+                              <TooltipContent>
+                                <Span className='Layer__tax-estimate__tooltip' size='sm'>{item.formula}</Span>


We don't have properties on our TextStyleProps to determine colors.

src/views/TaxFiling/taxEstimate.scss

I don't want to make design decisions for our buttons under crunch time, so I'd rather this be a specific class name that we later clean up (if we do decide to extend our Span component.

darrensapalo · 2025-11-14T01:33:40Z

src/views/TaxEstimates/taxEstimate.scss

+.Layer__tax-estimate__deduction-item {
+  padding-left: var(--spacing-xl);
+}


Spans currently do not have padding-inline (left, right) and only support padding-block (top, bottom).

Understandably, spans are inline objects, so there is a case to be made that spans shouldn't have padding-inline because that seems weird. I don't want to encourage Span users to use padding-inline if this is a usually unexpected styling approach.

Maybe the better thing to do is wrap a Span with a container, and then have the container manage the padding instead. In terms of DOM structure optimization & also library size, fewer VStack and HStack might be a better preference. Those containers add up, or dare I say... stack. 👀

darrensapalo · 2025-11-14T01:35:02Z

src/views/TaxEstimates/taxEstimateView.scss

@@ -0,0 +1,34 @@
+.Layer__tax-filing {
+  max-width: 1460px;


This magic number is the same magic number used for .Layer__view-header__content.

greptile-apps · 2025-11-14T01:37:21Z

Greptile Overview

Greptile Summary

This PR introduces a new experimental TaxFilingView component that provides tax estimation and filing functionality. The implementation includes:

New tax filing view with tabs for Overview, Tax Estimates, Tax Payments, and Tax Profile
Collapsible detail sections for Adjusted Gross Income, Federal Taxes, and State Taxes
Reducer-based state management for form data
Enhanced View/ViewHeader components with headerActions prop support
Improved dropdown menu disabled state styling

Key Changes:

Added 15 new files in src/views/TaxFiling/ directory
Component follows existing patterns and uses colocated styles
Types properly use type declarations instead of interface for data objects per custom rules
Exported as unstable_TaxFilingView to indicate experimental status

Style Issues:

Uses padding-left instead of RTL-aware padding-inline-start in one location (violates RTL rule)
Uses 1px instead of 0.0625rem for border widths in one location (violates rem rule)

Code Quality:

Well-structured component hierarchy with proper separation of concerns
Good use of TypeScript types and proper prop interfaces
State management follows React best practices with useReducer
No logical errors or security concerns identified

Confidence Score: 4/5

This PR is safe to merge with minimal risk - it's experimental groundwork with no logical errors
Score reflects well-structured code with only minor style guideline violations. The experimental nature (unstable_ prefix) appropriately signals this is groundwork. No logical errors, security issues, or data integrity concerns found. The two style issues (RTL padding and px vs rem) are non-critical and easily addressable.
Pay attention to src/views/TaxFiling/taxEstimate.scss for style guideline compliance before stabilizing the feature

Important Files Changed

File Analysis

Filename	Score	Overview
src/views/TaxFiling/taxEstimate.scss	3/5	Styling file with padding violations (uses padding-left instead of RTL-aware properties), height in rem (good), but missing some rem conversions
src/views/TaxFiling/TaxEstimate.tsx	4/5	Main tax estimate component with good structure, uses type definitions correctly, properly imports colocated styles
src/views/TaxFiling/TaxFilingView.tsx	4/5	Main view component with good state management, proper reducer usage, and well-organized tab structure
src/views/TaxFiling/AdjustedGrossIncomeSection.tsx	4/5	Table-based section component with tooltip support, good structure but could simplify nested ternaries
src/views/TaxFiling/FederalTaxesSection.tsx	4/5	Similar structure to AGI section, handles rates vs amounts correctly, complex nested ternaries
src/views/TaxFiling/StateTaxesSection.tsx	4/5	Duplicate of FederalTaxesSection with minor naming changes, potential for code reuse

Sequence Diagram

sequenceDiagram
    participant User
    participant TaxFilingView
    participant Toggle
    participant TaxEstimate
    participant Sections
    participant Store
    
    User->>TaxFilingView: Load View
    TaxFilingView->>Store: Initialize with reducer
    TaxFilingView->>Toggle: Render tabs
    
    alt User not onboarded
        TaxFilingView->>TaxFilingView: setActiveTab('tax-profile')
        User->>TaxFilingView: Fill profile form
        User->>TaxFilingView: Click Save
        TaxFilingView->>Store: Dispatch profile actions
        TaxFilingView->>TaxFilingView: setIsOnboarded(true)
        TaxFilingView->>TaxFilingView: setActiveTab('tax-estimates')
    end
    
    User->>Toggle: Select 'tax-estimates' tab
    Toggle->>TaxFilingView: onChange
    TaxFilingView->>TaxEstimate: Render with defaults
    TaxEstimate->>Sections: Render AGI, Federal, State sections
    
    User->>Sections: Click section to expand
    Sections->>TaxFilingView: onExpandedChange(true)
    TaxFilingView->>TaxFilingView: Update expansion state
    TaxFilingView->>Sections: Re-render with expanded=true
    Sections->>User: Show detailed breakdown
    
    User->>TaxEstimate: Click tax amount
    TaxEstimate->>TaxFilingView: onFederalTaxesOwedClick
    TaxFilingView->>TaxFilingView: setActiveTab, expand section
    TaxFilingView->>Sections: Re-render expanded section

greptile-apps

_{22 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

src/views/TaxFiling/taxEstimate.scss

greptile-apps · 2025-11-14T01:37:20Z

src/views/TaxEstimates/taxEstimate.scss

+  width: 15rem;
+  border-right: 1px solid var(--color-base-300);
+}


style: Use rem instead of px for the border-right width

Suggested change

width: 15rem;

border-right: 1px solid var(--color-base-300);

}

.Layer__tax-estimate__overview-total {

width: 15rem;

border-right: 0.0625rem solid var(--color-base-300);

}

Context Used: Rule from dashboard - When writing CSS styles, prefer rem instead of px usage. (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/views/TaxFiling/taxEstimate.scss Line: 20:22 Comment: **style:** Use `rem` instead of `px` for the border-right width ```suggestion .Layer__tax-estimate__overview-total { width: 15rem; border-right: 0.0625rem solid var(--color-base-300); } ``` **Context Used:** Rule from `dashboard` - When writing CSS styles, prefer `rem` instead of `px` usage. ([source](https://app.greptile.com/review/custom-context?memory=2d1130c2-7a17-4663-ac54-76dc142598ad)) How can I resolve this? If you propose a fix, please make it concise.

darrensapalo · 2025-11-20T23:04:56Z

@sarahraines Before reviewing this PR please check out the hooks first #1002 (integration branch) which would merge into this feature branch.

darrensapalo added 8 commits November 12, 2025 14:11

WIP for tax filing component

144adac

WIP for tax estimation

318ee08

Added new overview section

8157e8c

WIP for overview, tax estimates, tax payments

514489b

Fixed copy text issue on tax payments tab

bf61d45

Last polish for tax filing

2ae57cd

Cleaned up unused SCSS rules

68de537

Marked Tax Filing as unstable

e9396de

darrensapalo changed the title ~~darren/tax estimates~~ Unstable experimental build for TaxEstimateView Nov 13, 2025

darrensapalo added 2 commits November 13, 2025 14:04

Set a bit more color for our overview

dc6311b

Finished all of revisions from Justin

d64b0ed

doneel reviewed Nov 13, 2025

View reviewed changes

darrensapalo added 3 commits November 13, 2025 16:39

General improvements; Payment rework

ec861a3

Updated spacing for the charts

d4bc390

Cleaned up unused SCSS rules

52938d6

darrensapalo commented Nov 14, 2025

View reviewed changes

darrensapalo marked this pull request as ready for review November 14, 2025 01:35

greptile-apps bot reviewed Nov 14, 2025

View reviewed changes

darrensapalo added 5 commits November 13, 2025 17:44

Uses padding-inline-start instead of padding-left.

2163512

Package version bump to 114-alpha.2

4cc559b

Final polish for tax filing

116ae8d

Merge branch 'main' into darren/tax-estimates

24ed21a

Moved files to tax estimates instead of tax filing

f48fc1d

darrensapalo requested a review from sarahraines November 20, 2025 23:03

Unstable experimental build for TaxEstimateView #984

Are you sure you want to change the base?

Unstable experimental build for TaxEstimateView #984

Uh oh!

Conversation

darrensapalo commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Out of scope

Blockers

How this has been tested?

References

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

darrensapalo commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darrensapalo commented Nov 13, 2025 •

edited

Loading