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

Toast editor #10390

Merged
merged 6 commits into from
Jan 30, 2024
Merged

Toast editor #10390

merged 6 commits into from
Jan 30, 2024

Conversation

AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented Jan 22, 2024

Description

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

Open tasks:

  • ...

Copy link

update-docs bot commented Jan 22, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lookacat
Copy link
Contributor

lookacat commented Jan 23, 2024

The show more / collapse button for the spaces description is now fixed. Not sure if its good that we manually add a dependency from toast, it seems they forgot to add it

@AlexAndBear AlexAndBear force-pushed the toast-editor branch 4 times, most recently from 69d0439 to 3f6dc22 Compare January 25, 2024 14:10
Copy link
Member

@dschmidt dschmidt left a comment

Choose a reason for hiding this comment

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

great, just two minor nitpicks

package.json Outdated Show resolved Hide resolved
packages/web-pkg/src/components/Editor.vue Outdated Show resolved Hide resolved
@AlexAndBear
Copy link
Contributor Author

Quality Gate Failed Quality Gate failed

Failed conditions

31.91% Condition Coverage on New Code (required ≥ 50%)

See analysis details on SonarCloud

We have a lot of acceptance tests, ihmo that's fine

Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

Awesome stuff! 🚀 Only one small remark from my side.

packages/web-pkg/src/components/Editor.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Having some serious text editing issues:

  • blanks in .txt files are not accepted anymore (or at least not rendered within the editor)
  • moving the cursor to whatever location which is not the end of the document and then typing 1 character moves the cursor back to the end of the document. Which makes editing existing content impossible.
  • within a .md file, using cmd+s (on a mac, obviously) adds 4 tilde chars to the current cursor position ( ~~~~). Happens right before the actual document save, so probably a keyboard shortcut from the editor which needs to be disabled?

All of this happened in Chrome, hope I'm just too stupid 😅 Could you check please?

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Super awesome integration, love it! 🤩

packages/web-pkg/src/components/Editor.vue Outdated Show resolved Hide resolved
packages/web-pkg/src/components/Editor.vue Outdated Show resolved Hide resolved
packages/web-pkg/src/components/Editor.vue Outdated Show resolved Hide resolved
@dschmidt
Copy link
Member

Can you check the editor behaves if you switch from one file to another via search? (i.e. direct navigation from one file to another without leaving the editor)

I haven't tried it myself, just know that it can be tricky to setup the currentContent reactivity correctly when not using a native vue library

@dschmidt
Copy link
Member

Can you check the editor behaves if you switch from one file to another via search? (i.e. direct navigation from one file to another without leaving the editor)

I haven't tried it myself, just know that it can be tricky to setup the currentContent reactivity correctly when not using a native vue library

This is broken and cumbersome to fix, I would prefer a generic solution

Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

4 New issues
0 Security Hotspots
62.9% Coverage on New Code
1.5% Duplication on New Code

See analysis details on SonarCloud

@AlexAndBear AlexAndBear merged commit 9fe6a9d into master Jan 30, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the toast-editor branch January 30, 2024 18:19
@micbar micbar mentioned this pull request Jun 19, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web] Add Editor Toolbar to markdown editor Codeblocks in Markdown are not properly formated
5 participants