kwila/office-extensions
Watch
1
0
Fork
You've already forked office-extensions
 0
Code Issues Pull requests Projects Releases Packages Activity Actions

docs: UI improvement ideas #27

Merged
addison merged 22 commits from ui-improvement-ideas into main 2026-08-08 06:56:13 -04:00
Conversation 3 Commits 22 Files changed 10 +205 -1
addison commented 2026-07-17 17:17:34 -04:00
Owner
Copy link
No description provided.
docs: update spec 4 goals
All checks were successful
CI / frontend (pull_request) Successful in 26s
Details
CI / backend (pull_request) Successful in 37s
Details
59fd700508
addison changed title from docs: UI improvement ideas to WIP: docs: UI improvement ideas 2026-07-17 17:17:36 -04:00
addison commented 2026-07-17 17:18:07 -04:00
Author
Owner
Copy link

@talha if you have an hour or two, take a look at this project and give me any UI improvement ideas. the UI looks how we want it, but I'm sure the code could use a lot of clean up.

@talha if you have an hour or two, take a look at this project and give me any UI improvement ideas. the UI _looks_ how we want it, but I'm sure the code could use a lot of clean up.
addison requested review from talha 2026-07-17 17:18:19 -04:00
addison self-assigned this 2026-07-17 17:18:20 -04:00
talha commented 2026-07-18 07:07:13 -04:00
Collaborator
Copy link

Issues:

/App.tsx

  • Instead of putting every component inside Layout, we can add Layout in parent route and then adding each route as its child.
    i.e
    function AppLayout() {
    return (
    <Layout>
    <Outlet />
    </Layout>
    );
    }
    // existing code
    <Route element={<AppLayout />}>
    <Route path="/reports/job-commissions" element={<JobCommissionsPage />} />
    <Route path="/reports/sales" element={<SalesReportPage />} />
    // other routes
    </Route>

/components/CopyableCell.tsx

  • In current implementation, we aren;t clearing any existing timeout before applying timeout again, it can cause glitches.
  • Also on unmount of component we aren't clearing timout too, it can give react warnings as stray state can be getting updated.
    my suggestion for both:
  • use useRef to create reference to timout and on unmount clear timout and before setting another timout clear timout too.

/components/reports/job-commissions/JobCommissionsPage.tsx

  • methods getPreviousMonthDates and getMonthDates should be outside component definition, as we don't want them to be initialized again and again
  • also getPreviousMonthDates gets called on every initialization of component, we can call it inside useState as callback so it gets called only once
  • I really think, we don't need all these functions setSelectedEmployee, setBalanceStatus, setProfitLevel, setCommissionStatus and setWriteOffStatus, we can simply make a function updateFilter with definition like
    const updateFilter = <K extends keyof InvoiceFilters>(
    key: K,
    value: InvoiceFilters[K],
    ) => setFilters((prev) => ({ ...prev, [key]: value }));
    and then call it like
    updateFilter("selectedEmployeeID", value)
  • SImilarly getMonthOptions shouldn't be defined inside component too. it can be moved outside component, or it would be better to move it to some util as this file is already too long.
  • One more suggestion, useeffect with no dependency must be written first then other useeffects, like in our case fetchemployees useeffect must be at top of useeffect list
  • Shouldn't we show any error to user if fetch employees fail? like we are doing for invoices
  • while setting setFiltersExpanded it would be i think better to do this like setFiltersExpanded(prev => !prev) instead of setFiltersExpanded(!filtersExpanded)
  • One suggestion for this file is that, instead of sending requests on each filter update using debounce, it would be better to have search button and on its click we can send request.

/components/reports/job-commissions/JobCommissionsPrintPage.tsx

  • getPreviousMonthDates is static function and is defined here too, it's better to keep these functions in utils file and then use it where required.
  • 2nd useEffect with dependency as loading, error is unnecessary and it can be get rid off, by having hasError variable in fetchInvoices of first useEffect and setting it true if any error encountered, and in finally keyword where you set setLoading, just call window.print inside set timeout if hasError is false and in same useEffect's unMount clear timeout of window.print timeout

/components/reports/sales/SalesReportPage.tsx

  • Here we are fetching employees with all repeated code. i would recommend to create a helper function which returns sortedEmployees or error and as param we can send feature and abort controller, it would cause reduction of repeated logics and code. and probably for invoices too.
  • selectedEmployeeID in 2nd useEffect is redundant, as 1st useeffect has it too and it reinitializes fetchInvoices which runs 2nd useeffect too.

/components/reports/sales/SalesReportPrintPage.tsx

  • We can use fetch sorted employees function or invoice one here too.
  • 3nd useEffect with dependency as loading, error is unnecessary and it can be get rid off, by having hasError variable in fetchInvoices of first useEffect and setting it true if any error encountered, and in finally keyword where you set setLoading, just call window.print inside set timeout if hasError is false and in same useEffect's unMount clear timeout of window.print timeout

/components/configuration/employees/EmployeesPage.tsx

  • looks good
Issues: /App.tsx - Instead of putting every component inside Layout, we can add Layout in parent route and then adding each route as its child. i.e `function AppLayout() {` ` return (` ` <Layout>` ` <Outlet />` ` </Layout>` ` );` `}` `// existing code ` `<Route element={<AppLayout />}>` ` <Route path="/reports/job-commissions" element={<JobCommissionsPage />} />` ` <Route path="/reports/sales" element={<SalesReportPage />} />` ` // other routes` `</Route>` /components/CopyableCell.tsx - In current implementation, we aren;t clearing any existing timeout before applying timeout again, it can cause glitches. - Also on unmount of component we aren't clearing timout too, it can give react warnings as stray state can be getting updated. my suggestion for both: - use useRef to create reference to timout and on unmount clear timout and before setting another timout clear timout too. /components/reports/job-commissions/JobCommissionsPage.tsx - methods `getPreviousMonthDates` and `getMonthDates` should be outside component definition, as we don't want them to be initialized again and again - also `getPreviousMonthDates` gets called on every initialization of component, we can call it inside `useState` as callback so it gets called only once - I really think, we don't need all these functions `setSelectedEmployee`, `setBalanceStatus`, `setProfitLevel`, `setCommissionStatus` and `setWriteOffStatus`, we can simply make a function `updateFilter` with definition like `const updateFilter = <K extends keyof InvoiceFilters>(` ` key: K,` ` value: InvoiceFilters[K],` ` ) => setFilters((prev) => ({ ...prev, [key]: value }));` and then call it like `updateFilter("selectedEmployeeID", value)` - SImilarly `getMonthOptions` shouldn't be defined inside component too. it can be moved outside component, or it would be better to move it to some util as this file is already too long. - One more suggestion, useeffect with no dependency must be written first then other useeffects, like in our case fetchemployees useeffect must be at top of useeffect list - Shouldn't we show any error to user if fetch employees fail? like we are doing for invoices - while setting `setFiltersExpanded` it would be i think better to do this like `setFiltersExpanded(prev => !prev)` instead of `setFiltersExpanded(!filtersExpanded)` - One suggestion for this file is that, instead of sending requests on each filter update using debounce, it would be better to have search button and on its click we can send request. /components/reports/job-commissions/JobCommissionsPrintPage.tsx - `getPreviousMonthDates` is static function and is defined here too, it's better to keep these functions in utils file and then use it where required. - 2nd useEffect with dependency as loading, error is unnecessary and it can be get rid off, by having hasError variable in fetchInvoices of first useEffect and setting it true if any error encountered, and in finally keyword where you set setLoading, just call window.print inside set timeout if hasError is false and in same useEffect's unMount clear timeout of window.print timeout /components/reports/sales/SalesReportPage.tsx - Here we are fetching employees with all repeated code. i would recommend to create a helper function which returns sortedEmployees or error and as param we can send feature and abort controller, it would cause reduction of repeated logics and code. and probably for invoices too. - `selectedEmployeeID` in 2nd useEffect is redundant, as 1st useeffect has it too and it reinitializes `fetchInvoices` which runs 2nd useeffect too. /components/reports/sales/SalesReportPrintPage.tsx - We can use fetch sorted employees function or invoice one here too. - 3nd useEffect with dependency as loading, error is unnecessary and it can be get rid off, by having hasError variable in fetchInvoices of first useEffect and setting it true if any error encountered, and in finally keyword where you set setLoading, just call window.print inside set timeout if hasError is false and in same useEffect's unMount clear timeout of window.print timeout /components/configuration/employees/EmployeesPage.tsx - looks good
addison commented 2026-07-18 07:09:14 -04:00
Author
Owner
Copy link

Thanks! I will dig into adding some of these to specs 👍

Thanks! I will dig into adding some of these to specs 👍
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

# Conflicts:
#	.agents/setup
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
docs: consolidate reusable filter goals
Some checks were skipped
Amp Review / review (pull_request) Has been skipped
Details
73aefeea7f 
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
addison changed title from WIP: docs: UI improvement ideas to docs: UI improvement ideas 2026-07-28 06:01:00 -04:00
addison changed title from docs: UI improvement ideas to WIP: docs: UI improvement ideas 2026-07-28 06:01:43 -04:00 
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
chore: install Go in orb setup
Some checks were skipped
Amp Review / review (pull_request) Has been skipped
Details
cad85659c5 
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
addison changed title from WIP: docs: UI improvement ideas to docs: UI improvement ideas 2026-07-28 06:18:46 -04:00
addison changed title from docs: UI improvement ideas to WIP: docs: UI improvement ideas 2026-07-28 06:19:04 -04:00
addison changed title from WIP: docs: UI improvement ideas to docs: UI improvement ideas 2026-07-28 06:19:05 -04:00
addison changed title from docs: UI improvement ideas to WIP: docs: UI improvement ideas 2026-07-28 06:19:10 -04:00 
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
docs: approve application routing spec
All checks were successful
Amp Review / review (pull_request) Successful in 1s
Details
afd6e30994 
Amp-Thread-ID: https://ampcode.com/threads/T-019fa815-b3a9-77e3-a013-649d1e68648f

Co-authored-by: Addison Emig <[email protected]>
addison changed title from WIP: docs: UI improvement ideas to docs: UI improvement ideas 2026-08-08 06:55:58 -04:00
addison deleted branch ui-improvement-ideas 2026-08-08 06:56:13 -04:00
addison referenced this pull request from a commit 2026-08-08 06:56:15 -04:00
docs: UI improvement ideas (#27)
Sign in to join this conversation.
Reviewers
No reviewers
talha
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
addison
talha
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kwila/office-extensions!27
No description provided.