Format transaction notes as clickable tags #2670

joel-jeremy · 2024-04-26T15:45:23Z

netlify · 2024-04-26T15:45:40Z

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	`f0429b0`
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/664e75b51e67e50008d97e85
😎 Deploy Preview	https://deploy-preview-2670.demo.actualbudget.org
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2024-04-26T15:47:50Z

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
9	4.71 MB → 5.25 MB (+551.44 kB)	+11.42%

Changeset

File	Δ	Size
`node_modules/lodash/lodash.js`	🆕 +547.05 kB	0 B → 547.05 kB
`src/components/filters/ConditionsOpMenu.tsx`	🆕 +703 B	0 B → 703 B
`node_modules/lodash/lodash.js?commonjs-module`	🆕 +27 B	0 B → 27 B
`src/hooks/useFilters.ts`	📈 +158 B (+10.74%)	1.44 kB → 1.59 kB
`src/components/reports/graphs/tableGraph/ReportTableRow.tsx`	📈 +554 B (+8.30%)	6.52 kB → 7.06 kB
`src/components/transactions/TransactionList.jsx`	📈 +287 B (+6.83%)	4.11 kB → 4.39 kB
`src/components/filters/AppliedFilters.tsx`	📈 +37 B (+4.82%)	767 B → 804 B
`src/components/filters/updateFilterReducer.ts`	📈 +27 B (+3.76%)	719 B → 746 B
`src/components/transactions/TransactionsTable.jsx`	📈 +1.96 kB (+3.67%)	53.32 kB → 55.27 kB
`src/components/filters/FiltersStack.tsx`	📈 +23 B (+2.37%)	971 B → 994 B
`src/style/themes/development.ts`	📈 +116 B (+1.63%)	6.95 kB → 7.06 kB
`src/style/themes/dark.ts`	📈 +116 B (+1.62%)	7 kB → 7.11 kB
`src/style/themes/light.ts`	📈 +116 B (+1.60%)	7.08 kB → 7.19 kB
`src/style/themes/midnight.ts`	📈 +110 B (+1.59%)	6.76 kB → 6.86 kB
`home/runner/work/actual/actual/packages/loot-core/src/shared/rules.ts`	📈 +78 B (+1.32%)	5.77 kB → 5.85 kB
`src/components/accounts/Account.jsx`	📈 +504 B (+1.10%)	44.75 kB → 45.24 kB
`src/components/reports/reports/NetWorth.jsx`	📈 +33 B (+0.73%)	4.42 kB → 4.45 kB
`src/components/accounts/Header.jsx`	📈 +105 B (+0.68%)	14.98 kB → 15.08 kB
`src/components/reports/Header.jsx`	📈 +24 B (+0.58%)	4.06 kB → 4.08 kB
`src/components/reports/reports/CashFlow.tsx`	📈 +33 B (+0.47%)	6.79 kB → 6.82 kB
`src/components/reports/reports/Spending.tsx`	📈 +21 B (+0.24%)	8.6 kB → 8.62 kB
`src/components/table.tsx`	📈 +50 B (+0.20%)	24.26 kB → 24.31 kB
`src/components/reports/graphs/LineGraph.tsx`	📈 +12 B (+0.17%)	7.01 kB → 7.02 kB
`src/components/reports/reports/CustomReport.tsx`	📈 +33 B (+0.17%)	19.49 kB → 19.53 kB
`src/components/filters/FiltersMenu.jsx`	📈 +20 B (+0.16%)	12.41 kB → 12.43 kB
`src/components/reports/graphs/StackedBarGraph.tsx`	📈 +12 B (+0.15%)	7.75 kB → 7.76 kB
`src/components/reports/graphs/DonutGraph.tsx`	📈 +12 B (+0.14%)	8.29 kB → 8.31 kB
`src/components/reports/graphs/BarGraph.tsx`	📈 +12 B (+0.13%)	9.16 kB → 9.18 kB
`src/components/budget/index.tsx`	📈 +12 B (+0.13%)	9.17 kB → 9.18 kB
`src/components/modals/EditRule.jsx`	📈 +20 B (+0.06%)	34.5 kB → 34.52 kB
`src/components/accounts/Balance.jsx`	📈 +3 B (+0.05%)	5.39 kB → 5.39 kB
`src/components/filters/SavedFilterMenuButton.tsx`	📉 -8 B (-0.16%)	4.83 kB → 4.82 kB
`src/components/filters/FilterExpression.tsx`	📉 -61 B (-2.04%)	2.92 kB → 2.86 kB
`src/components/filters/CondOpMenu.tsx`	🔥 -700 B (-100%)	700 B → 0 B

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
static/js/wide.js	262.45 kB → 812.39 kB (+549.94 kB)	+209.54%
static/js/ReportRouter.js	1.23 MB → 1.23 MB (+904 B)	+0.07%
static/js/index.js	3 MB → 3 MB (+606 B)	+0.02%
static/js/AppliedFilters.js	20.41 kB → 20.44 kB (+26 B)	+0.12%

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
static/js/indexeddb-main-thread-worker-e59fee74.js	13.5 kB	0%
static/js/resize-observer.js	18.37 kB	0%
static/js/BackgroundImage.js	122.29 kB	0%
static/js/usePreviewTransactions.js	790 B	0%
static/js/narrow.js	59.97 kB	0%

github-actions · 2024-04-26T15:47:51Z

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
1	1.2 MB → 1.2 MB (+242 B)	+0.02%

Changeset

File	Δ	Size
`packages/loot-core/src/platform/server/sqlite/index.web.ts`	📈 +91 B (+1.74%)	5.1 kB → 5.18 kB
`packages/loot-core/src/shared/rules.ts`	📈 +100 B (+1.23%)	7.92 kB → 8.02 kB
`packages/loot-core/src/server/accounts/transaction-rules.ts`	📈 +266 B (+0.95%)	27.21 kB → 27.47 kB
`packages/loot-core/src/server/aql/compiler.ts`	📈 +309 B (+0.86%)	34.91 kB → 35.21 kB
`packages/loot-core/src/server/accounts/rules.ts`	📈 +82 B (+0.28%)	28.79 kB → 28.87 kB
`packages/loot-core/src/server/db/index.ts`	📉 -117 B (-0.65%)	17.71 kB → 17.59 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
kcab.worker.js	1.2 MB → 1.2 MB (+242 B)	+0.02%

Smaller

No assets were smaller

Unchanged

No assets were unchanged

youngcw · 2024-04-26T16:10:21Z

The colors probably could use some work, but other than that this looks great

joel-jeremy · 2024-04-26T16:56:36Z

Any suggestions on the colors? Would it be better to apply primary colors to them? But that would make the tags stand out on the table.

youngcw · 2024-04-26T17:18:35Z

I don't think I have good recommendations for specific colors. What I was seeing is that the pill colors can be hard to see when the transaction is hovered and highlighted (the same color in midnight mode), and the pill hover color was usually really close to the normal color so it was like nothing happened when hovering.

youngcw · 2024-04-26T20:54:10Z

Those colors look good to me

Teprifer · 2024-04-27T01:16:45Z

If you're already on the All Accounts screen, clicking the tag doesn't apply the filtering.

joel-jeremy · 2024-04-27T02:54:18Z

If you're already on the All Accounts screen, clicking the tag doesn't apply the filtering.
Should be fixed now :)

Teprifer · 2024-04-27T13:13:38Z

Thanks! :)

I also like the new behaviour where it only filters the current account screen when clicked on, I think it's more intuitive.

MatissJanis · 2024-04-27T21:01:28Z

Maybe it's just me, but I dislike that the tags are in bold. Nothing else in the table is in bold.

Other than that: this is an amazing addition! Thanks for working on this!

MatissJanis · 2024-04-29T19:08:38Z

packages/desktop-client/src/components/transactions/TransactionList.jsx

+    const filterConditions = [
+      { field: 'notes', op: 'contains', value: noteTag, type: 'string' },
+    ];
+    onApplyFilters(filterConditions);


💭 thought: ‏currently the click on a tag REPLACES the existing filters. Would it be better to instead append a filter?

I can see a use case where I filter by category. And then click on a tag to narrow it down even further. Currently that would remove the category filter.

WDYT?

MatissJanis · 2024-04-29T19:10:10Z

packages/desktop-client/src/components/transactions/TransactionsTable.jsx

+function notesTagFormatter(value, onNoteTagClick) {
+  const words = value.split(' ');
+  return (
+    <>


🥜 nitpick: ‏unnecessary wrapping fragment.

carkom

First of all this is amazing and I love the functionality. Any chance we could add an autocomplete list to the tags?

There's a lot going on in this PR.

I'm not sure why changing naming conventions for filters across the entire app is relevant for this PR, would be better as a separate PR.

I like the tag colors, they look good to me. Pill color changes are no good. They are used in 4 other pages and the proposed changes do not pass AA (eg they are harder to read). Might also be best in a different PR as the pill color doesn't really relate to the core functionality of this PR.

packages/desktop-client/src/style/themes/dark.ts

youngcw · 2024-04-29T22:51:35Z

If tags have overlap in their names then you get multiple tags in the filter. Example: make tags for #tag and #tag2 and then click on #tag and both show up in the filtered list

...actions.test.js-snapshots/Transactions-filters-transactions-by-category-7-chromium-linux.png

youngcw · 2024-05-21T21:06:49Z

That's works great when for searching all tags. But it doesn't seem to work when searching for a single tag.

Wouldn't the existing #tag/b still work if that other expression was used to mark the tags?

psybers · 2024-05-21T21:06:59Z

I think the regex needs to be something like (^|\s)#Test\b. So it ends on a word boundary but starts at either the beginning of the note or a space.

youngcw · 2024-05-21T21:15:08Z

I guess maybe we need to nail down what we want to have count as a tag and what is allowed in a tag. After that its pretty simple to make a regex to match those rules we want to enforce

joel-jeremy · 2024-05-21T22:44:29Z

I think the regex needs to be something like (^|\s)#Test\b. So it ends on a word boundary but starts at either the beginning of the note or a space.

This seem to work great:

I guess maybe we need to nail down what we want to have count as a tag and what is allowed in a tag. After that its pretty simple to make a regex to match those rules we want to enforce

I would say we should just allow any characters in tags

psybers · 2024-05-22T00:59:20Z

Seems to be a bug. If you select two tags, it is set to all yet it isn't behaving that way.

If you change it to any and back to all you see the transaction list is not the same.

psybers · 2024-05-22T01:00:33Z

Also, weird tags break things:

Clicking that you get nothing matched.

psybers · 2024-05-22T01:01:19Z

Probably it is because of the regex characters in it - you might need to escape the tag when building the regex?

joel-jeremy · 2024-05-22T15:16:02Z

@psybers I have pushed a commit which escapes the regex. Tested some special characters and it seem to work fine expect when using ). It won't show any results if tag contains ) for some reason, even if it's escaped.

psybers · 2024-05-22T15:17:23Z

So the question still remains, is this what we want or not?

With \b on the right side, it will match #test#that (because '#' is a word boundary).

psybers · 2024-05-22T15:18:59Z

If we want only "whole tags" and consider that to mean whitespace delineated, then the regex could be (^|\s)#test($|\s).

joel-jeremy · 2024-05-22T15:30:30Z

If we want only "whole tags" and consider that to mean whitespace delineated, then the regex could be (^|\s)#test($|\s).

Yeah, when clicking tags, this is how I would expect the filters to behave. (^|\s)#test($|\s) seems to work well.

youngcw · 2024-05-22T15:46:15Z

I still don't like that #test#tag would count as one long tag. It probably would make more sense to have that parse as not a tag if we are going to assume a space at the end for the search.

psybers · 2024-05-22T16:11:32Z

So there are three options for #test#tag:

Treat it as two, separate tags.
Treat it as a single big tag.
Ignore it. It doesn't look like a tag, so don't turn it into a button.

Is there a commonly accepted answer used by many other software? If so I would say mimic the common behavior. If not, my vote is for 1.

The current solution is 2. If we decide to switch to one of the other solutions, the regex will probably need updated too.

youngcw · 2024-05-22T16:23:25Z

So there are three options for #test#tag:

Treat it as two, separate tags.

Treat it as a single big tag.

Ignore it. It doesn't look like a tag, so don't turn it into a button.

I would vote for option 1. I think that would be the most obvious to people and we can easily parse that with /b in a regex. Finding the tags should be pretty straight forward to. Look for a #, end the tag when there is another # or a space. The searching regex would have to escape most symbols if they are allowed

psybers · 2024-05-22T16:27:22Z

For what its worth, 1 is how Facebook behaves.

This is two separate tags.

joel-jeremy · 2024-05-22T20:22:15Z

@psybers @youngcw Pushed a change to treat as separate tags

psybers · 2024-05-22T20:31:15Z

This behavior seems good to me.

youngcw · 2024-05-22T20:32:46Z

That seems to be working really well.

Some things of note:

$ in a tag breaks the search. I can't tell why since it does get escaped. All other symbols I have seem to work fine
A single # at the end of a tag disappears, not the end of the world, but a bit strange. It doesn't show up as a tag, which is fine, but it doesn't show in the rendered note either.

psybers · 2024-05-22T20:39:00Z

Here is a weird one: test#

Turns it into #test.

psybers · 2024-05-22T20:39:37Z

And possibly related? foo#bar becomes #foo #bar

joel-jeremy · 2024-05-22T22:46:48Z

Updated the rendering logic of tags. The current regex doesn't seem work for tags that are in the middle of a string.

psybers · 2024-05-22T23:55:49Z

Might have to allow it to be bordered by another tag... maybe something like this?

(^|\s|\w)#test($|\s|#)

joel-jeremy · 2024-05-24T15:20:04Z

@psybers The problem string is when tag is preceded by a # e.g. Onelongwordinnoteswith##tag. This is not being matched by the current and the new regex you provided.

psybers · 2024-05-24T15:27:55Z

@joel-jeremy I mean the simple fix is to add # on the left side. (^|\s|\w|#)#test($|\s|#)

But this regex is feeling more and more hacky and I worry it will match things we do not intend. The best approach would be to know a more or less complete set of all cases we want to match (and cases to avoid), build the NFA by hand, and then turn it into a regex.

trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Apr 26, 2024

github-actions bot changed the title ~~Format transaction notes as clickable tags~~ [WIP] Format transaction notes as clickable tags Apr 26, 2024

trafico-bot bot added 🚧 WIP Still work-in-progress, please don't review and don't merge and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Apr 26, 2024

joel-jeremy changed the title ~~[WIP] Format transaction notes as clickable tags~~ Format transaction notes as clickable tags Apr 26, 2024

trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed 🚧 WIP Still work-in-progress, please don't review and don't merge labels Apr 26, 2024

joel-jeremy force-pushed the transaction-notes-as-tags branch from 488b421 to eea961a Compare April 27, 2024 21:23

MatissJanis previously approved these changes Apr 29, 2024

View reviewed changes

trafico-bot bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Apr 29, 2024

carkom requested changes Apr 29, 2024

View reviewed changes

packages/desktop-client/src/style/themes/dark.ts Outdated Show resolved Hide resolved

trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed ✅ Approved Pull Request has been approved and can be merged labels Apr 29, 2024

joel-jeremy dismissed MatissJanis’s stale review via 6f1b66f April 29, 2024 22:12

trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Apr 29, 2024

youngcw reviewed Apr 30, 2024

View reviewed changes

...actions.test.js-snapshots/Transactions-filters-transactions-by-category-7-chromium-linux.png Outdated Show resolved Hide resolved

Update tag regex

17068eb

Escape regex input

9ce8a31

Update tag regex

891f2b6

joel-jeremy added 2 commits May 22, 2024 12:46

Use onApplyFilter

da2a819

Update tag formatting

f97afcb

Fix tag formatting

f0429b0

Format transaction notes as clickable tags #2670

Are you sure you want to change the base?

Format transaction notes as clickable tags #2670

Conversation

joel-jeremy commented Apr 26, 2024

netlify bot commented Apr 26, 2024 • edited

✅ Deploy Preview for actualbudget ready!

github-actions bot commented Apr 26, 2024 • edited

Bundle Stats — desktop-client

github-actions bot commented Apr 26, 2024 • edited

Bundle Stats — loot-core

youngcw commented Apr 26, 2024

joel-jeremy commented Apr 26, 2024

youngcw commented Apr 26, 2024

youngcw commented Apr 26, 2024

Teprifer commented Apr 27, 2024

joel-jeremy commented Apr 27, 2024

Teprifer commented Apr 27, 2024

MatissJanis commented Apr 27, 2024

MatissJanis Apr 29, 2024

Choose a reason for hiding this comment

MatissJanis Apr 29, 2024

Choose a reason for hiding this comment

carkom left a comment • edited

Choose a reason for hiding this comment

youngcw commented Apr 29, 2024 • edited

youngcw commented May 21, 2024

psybers commented May 21, 2024

youngcw commented May 21, 2024

joel-jeremy commented May 21, 2024

psybers commented May 22, 2024

psybers commented May 22, 2024

psybers commented May 22, 2024

joel-jeremy commented May 22, 2024 • edited

psybers commented May 22, 2024

psybers commented May 22, 2024

joel-jeremy commented May 22, 2024

youngcw commented May 22, 2024

psybers commented May 22, 2024

youngcw commented May 22, 2024

psybers commented May 22, 2024

joel-jeremy commented May 22, 2024

psybers commented May 22, 2024

youngcw commented May 22, 2024

psybers commented May 22, 2024

psybers commented May 22, 2024

joel-jeremy commented May 22, 2024

psybers commented May 22, 2024

joel-jeremy commented May 24, 2024

psybers commented May 24, 2024 • edited

netlify bot commented Apr 26, 2024 •

edited

github-actions bot commented Apr 26, 2024 •

edited

github-actions bot commented Apr 26, 2024 •

edited

carkom left a comment •

edited

youngcw commented Apr 29, 2024 •

edited

joel-jeremy commented May 22, 2024 •

edited

psybers commented May 24, 2024 •

edited