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

Misc improvements including ALLOW_FILES and NEW_NOTE_NOTICE #119

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

KTamas
Copy link
Contributor

@KTamas KTamas commented Mar 15, 2024

Hi,

We at Compass really like this project and are planning to deploy a version ourselves. We need a few extra configurable features that I hope will mesh well with upstream.

The changes in the PR are:

  • Fix an error with file-saver being a CommonJS module
  • Dockerfile linting fixes (with Hadolint)
  • Add an ALLOW_FILES flag which lets the user disable file uploads
  • Update @types/node
  • Add a NEW_NOTE_NOTICE flag which lets you hide the notice about it being stored in the RAM etc (this would just confuse non-technical users)
  • Clicking the logo should always trigger a reset, which is important after you create a note. After copying the link, it's natural to just click the logo to get back to the "homepage"

Let me know if you want any changes/improvements/if this doesn't fit well with upstream/etc.

Best
Tamas

P.S: There may be more to come but this is a good first batch.

@cupcakearmy
Copy link
Owner

Thats super nice! Thanks 🚀 will have a look the coming days.
One thing I already noticed: I think NEW_NOTE_NOTICE should be THEME_NEW_NOTE_NOTICE, as all the other envs are related to cosmetic changes

@@ -3,7 +3,8 @@
</script>
<script lang="ts">
import { saveAs } from 'file-saver'
import pkg from 'file-saver'
Copy link
Owner

@cupcakearmy cupcakearmy Mar 18, 2024

Choose a reason for hiding this comment

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

maybe we could inline the actual import, it's only one file. But I can do it in a later time

Copy link
Contributor Author

@KTamas KTamas Mar 19, 2024

Choose a reason for hiding this comment

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

It would be nice but as far as I know, there is no way to make it one line as long as file-saver is CJS (in its current form). It seems like that package hasn't been updated for 3 years now.

Copy link
Owner

Choose a reason for hiding this comment

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

I mean like removing the dependency, and just copy pasta the actual library into the code base. It's 1 file. But i'll do it in another PR :)

@KTamas
Copy link
Contributor Author

KTamas commented Mar 19, 2024

@cupcakearmy updated the name of the flag!

@cupcakearmy cupcakearmy merged commit 9b48d39 into cupcakearmy:main Mar 19, 2024
1 check passed
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.

None yet

2 participants