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(files): create suggestions bar #6856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Jan 15, 2025

📝 Summary

🖼️ Screenshots

🏚️ Before

Screenshot from 2025-01-30 14-46-31

🏡 After

Screenshot from 2025-01-30 14-46-12
Peek 2025-01-30 14-43

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 12.82051% with 136 lines in your changes missing coverage. Please review.

Project coverage is 46.61%. Comparing base (6136301) to head (d8a7b03).

Files with missing lines Patch % Lines
src/components/SuggestionsBar.vue 0.00% 91 Missing and 1 partial ⚠️
src/marks/Link.js 31.11% 31 Missing ⚠️
src/helpers/filePicker.js 41.66% 7 Missing ⚠️
src/components/Editor.vue 0.00% 4 Missing ⚠️
src/components/Menu/ActionInsertLink.vue 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6856      +/-   ##
==========================================
- Coverage   46.68%   46.61%   -0.07%     
==========================================
  Files         750      730      -20     
  Lines       34528    34615      +87     
  Branches     1269     1249      -20     
==========================================
+ Hits        16118    16137      +19     
- Misses      17788    17877      +89     
+ Partials      622      601      -21     

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

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/6817-Suggestions-on-smart-picking branch from a1dc138 to ebc2d27 Compare January 16, 2025 15:47
@@ -612,6 +616,7 @@ export default {
this.emit('update:content', {
markdown: proseMirrorMarkdown,
})
this.isEmptyContent = editor.state.doc.nodeSize <= 4
Copy link
Member

@juliusknorr juliusknorr Jan 20, 2025

Choose a reason for hiding this comment

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

What is the background for that fixed number of 4? I'd assume we can also check the text content length directly? Or the resulting markdown length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

text content length is checked by the count of nodeSize from editor.state.doc, checked with Max

Copy link
Member

@juliusknorr juliusknorr Jan 30, 2025

Choose a reason for hiding this comment

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

text content length is checked by the count of nodeSize from editor.state.doc

Yes, that is what the code does. I would still be curious about the reasoning, this is not obvious. I assume there is one ...

@@ -58,6 +58,7 @@
</template>
<ContentContainer v-show="contentLoaded"
ref="contentWrapper" />
<SuggestionsBar v-if="isEmptyContent && contentLoaded" />
Copy link
Member

Choose a reason for hiding this comment

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

Should only be shown if a markdown file is edited, not for plain text files (e.g. .txt)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from the call i understood that SuggestionsBar should be shown if markdown file is not edited (empty). What should i use instead?

Copy link
Member

Choose a reason for hiding this comment

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

If you create a .txt file or upload any other code file (like .js or .php) and open it we will not have any rich text editing, so the options there do not make sense. There is also no menu bar. Let's just not show any suggestions in this case.

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/6817-Suggestions-on-smart-picking branch 2 times, most recently from bc78ba4 to bcbb1d0 Compare January 30, 2025 13:44
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review January 30, 2025 13:44
@JuliaKirschenheuter
Copy link
Contributor Author

/rebase

@JuliaKirschenheuter
Copy link
Contributor Author

@marcoambrosini could you please have a look into current state? Thank you!

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/6817-Suggestions-on-smart-picking branch from bcbb1d0 to 1c4e1c3 Compare January 31, 2025 09:34
@juliusknorr
Copy link
Member

@JuliaKirschenheuter No need to commit the bundle js files in this repo, they get updated once merged.

Signed-off-by: julia.kirschenheuter <[email protected]>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the enh/6817-Suggestions-on-smart-picking branch from 1c4e1c3 to d8a7b03 Compare January 31, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestions on smart picking
2 participants