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

Notes app #63

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open

Notes app #63

wants to merge 41 commits into from

Conversation

jaswch
Copy link

@jaswch jaswch commented Dec 25, 2022

No description provided.

@vercel
Copy link

vercel bot commented Dec 25, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
macos-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 15, 2023 4:53am

@PuruVJ
Copy link
Owner

PuruVJ commented Dec 25, 2022

Thanks for this! There area few errors though. Can you run the svelte-check command locally and fix them?

@jaswch
Copy link
Author

jaswch commented Dec 27, 2022

This error was solved by removing this attribute:

Before: <script lang="ts">
After: <script>

@PuruVJ
Copy link
Owner

PuruVJ commented Dec 27, 2022

<script lang="ts"> is necessary for keeping up the quality of the project. Please change it back to `ts` and fix the TypeScript errors :)

@jaswch
Copy link
Author

jaswch commented Dec 27, 2022

But by keeping <script lang="ts"> creates this problem:

Screenshot_select-area_20221227173702

Copy link
Owner

@PuruVJ PuruVJ left a comment

Choose a reason for hiding this comment

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

Solid work!

I have a few nitpicks, and a major request to change the confirm part to use the SystemDialog component, rather than confirm

src/components/apps/AppNexus.svelte Outdated Show resolved Hide resolved
src/components/apps/Notes/Notes.svelte Outdated Show resolved Hide resolved
src/components/apps/Notes/Notes.svelte Show resolved Hide resolved
src/components/apps/Notes/Notes.svelte Show resolved Hide resolved
src/components/apps/Notes/Notes.svelte Outdated Show resolved Hide resolved
src/components/apps/Notes/stores.js Outdated Show resolved Hide resolved
src/components/apps/Notes/stores.js Outdated Show resolved Hide resolved
src/components/apps/Notes/stores.js Outdated Show resolved Hide resolved
@jaswch
Copy link
Author

jaswch commented Mar 22, 2023

I have made the button's icons smaller and added a bar at the bottom of the title bar
Screenshot from 2023-03-22 13-40-02

@jaswch jaswch changed the title Notes app Notes app and Finder May 7, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jul 14, 2023

⚠️ No Changeset found

Latest commit: 3da7a40

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jaswch jaswch changed the title Notes app and Finder Notes app Jul 15, 2023
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.

2 participants