Skip to content

feat(core): at menu journal entries#8877

Closed
pengx17 wants to merge 1 commit intocanaryfrom
11-20-fix_core_at_menu_performance_on_large_doc_list
Closed

feat(core): at menu journal entries#8877
pengx17 wants to merge 1 commit intocanaryfrom
11-20-fix_core_at_menu_performance_on_large_doc_list

Conversation

@pengx17
Copy link
Collaborator

@pengx17 pengx17 commented Nov 20, 2024 •
edited
Loading

performance improvement:

fix AF-1790
When at menu appears or user query changes, the getMenu function will recomputed the title & icon of every doc in the workspace. In the original implementation, this will create a lot of adhoc LiveData instances but their values are never watched. The overhead is significant if there are many docs in the workspace.

This PR remove LiveData uses in this specific case. Also breaks down the long sync task for smoother user interaction.

depends on toeverything/blocksuite#8780

journal enhancement

fix AF-1774
added DateSelectorDialog for selecting a date through blocksuite;
added AtMenuConfigService for constructing the at menu config

e2e tests

fix AF-1776
fix PD-1942

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 20, 2024

Your org has enabled the Graphite merge queue for merging into canary

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “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 mod:infra Environment related issues and discussions app:core labels Nov 20, 2024
@pengx17 Graphite App
Copy link
Collaborator Author

pengx17 commented Nov 20, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pengx17 pengx17 marked this pull request as ready for review November 20, 2024 13:58
@pengx17 pengx17 force-pushed the 11-20-fix_core_at_menu_performance_on_large_doc_list branch from c9a5c4a to 29a8b94 Compare November 20, 2024 14:03
@nx-cloud
Copy link

nx-cloud bot commented Nov 20, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7161d02. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@pengx17 pengx17 force-pushed the 11-20-fix_core_at_menu_performance_on_large_doc_list branch from 29a8b94 to 7c6c2e1 Compare November 21, 2024 01:49
@codecov
Copy link

codecov bot commented Nov 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 27.43902% with 119 lines in your changes missing coverage. Please review.

Project coverage is 64.89%. Comparing base (e3a8f1e) to head (7161d02).

Files with missing lines Patch % Lines
.../core/src/modules/at-menu-config/services/index.ts 29.93% 102 Missing and 1 partial ⚠️
...ntend/core/src/modules/journal/services/journal.ts 5.88% 16 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           canary    #8877      +/-   ##
==========================================
- Coverage   70.54%   64.89%   -5.65%     
==========================================
  Files         544      659     +115     
  Lines       33772    37311    +3539     
  Branches     3008     3596     +588     
==========================================
+ Hits        23823    24213     +390     
- Misses       9585    12730    +3145     
- Partials      364      368       +4
Flag Coverage Δ
server-test 77.13% <ø> (ø)
unittest 35.01% <27.43%> (-11.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

graphite-app[bot]
graphite-app bot reviewed Nov 21, 2024
View reviewed changes
EYHN
EYHN reviewed Nov 21, 2024
View reviewed changes
EYHN
EYHN reviewed Nov 21, 2024
View reviewed changes
@pengx17 pengx17 force-pushed the 11-20-fix_core_at_menu_performance_on_large_doc_list branch from 7c6c2e1 to af7ecc5 Compare November 21, 2024 09:39
graphite-app[bot]
graphite-app bot reviewed Nov 21, 2024
View reviewed changes
@pengx17 pengx17 marked this pull request as ready for review November 21, 2024 17:02
@pengx17 pengx17 force-pushed the 11-20-fix_core_at_menu_performance_on_large_doc_list branch from d1e6694 to d800062 Compare November 22, 2024 05:27
@pengx17 pengx17 changed the title fix(core): at menu performance on large doc list feat(core): at menu enhancement (perf + journal date) Nov 22, 2024
@pengx17 pengx17 force-pushed the 11-20-fix_core_at_menu_performance_on_large_doc_list branch from d800062 to 652b7e1 Compare November 23, 2024 06:09
@pengx17 pengx17 changed the base branch from canary to renovate/blocksuite November 23, 2024 06:09
@pengx17 pengx17 mentioned this pull request Nov 23, 2024
Merged
1 task
@perfsee
Copy link

perfsee bot commented Nov 23, 2024 •
edited
Loading

affine-toeverything

Bundle main

diff ------------------- Bundle Size Diff -------------------------

@@                        EntryPoint: app                        @@
##                     canary …arge_doc_list                 +/- ##
===================================================================
< Bundle              22.9 MB        23.5 MB     +637 kB(+2.79%)   
< Initial JS          9.64 MB        9.98 MB     +344 kB(+3.57%)   
< Initial CSS          199 kB         248 kB   +48.3 kB(+24.22%)   
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#
< Assets                   76             82                  +6   
< Chunks                   74             80                  +6   
= Packages                294            294                       
= Duplicates                7              7                       
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Warnings ~~~~~~~~~~~~~~~~~~~~~~~~~~~#
! Deduplicate versions of libraries                                
! Separate mixed content assets files                              
! Avoid cache wasting

@pengx17 pengx17 mentioned this pull request Nov 25, 2024
Merged
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 27, 2024 •
edited
Loading

Merge activity

  • Nov 26, 9:01 PM EST: A user added this pull request to the Graphite merge queue.
  • Nov 26, 9:16 PM EST: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (All comments must be resolved before merging).
  • Nov 26, 9:38 PM EST: A user added this pull request to the Graphite merge queue.
  • Nov 26, 9:38 PM EST: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (All comments must be resolved before merging).
  • Nov 26, 9:39 PM EST: A user added this pull request to the Graphite merge queue.
  • Nov 26, 9:39 PM EST: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (All comments must be resolved before merging).

@pengx17
feat(core): at menu journal entries (#8877)
7161d02 
### performance improvement:

fix AF-1790
When at menu appears or user query changes, the `getMenu` function will recomputed the title & icon of every doc in the workspace. In the original implementation, this will create a lot of adhoc LiveData instances but their values are never watched. The overhead is significant if there are many docs in the workspace.

This PR remove LiveData uses in this specific case. Also breaks down the long sync task for smoother user interaction.

depends on toeverything/blocksuite#8780

### journal enhancement

fix AF-1774
added `DateSelectorDialog` for selecting a date through blocksuite;
added `AtMenuConfigService` for constructing the at menu config

### e2e tests
fix AF-1776
fix PD-1942
@pengx17 pengx17 mentioned this pull request Nov 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@forehalo forehalo forehalo approved these changes

@EYHN EYHN EYHN approved these changes

@JimmFly JimmFly Awaiting requested review from JimmFly

@CatsJuice CatsJuice Awaiting requested review from CatsJuice

+1 more reviewer

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

app:core mod:dev mod:i18n Related to i18n mod:infra Environment related issues and discussions test Related to test cases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pengx17 @forehalo @EYHN @CatsJuice