Skip to content

Allow naming zip as prefix#14

Open
xllifi wants to merge 4 commits intoRuyiLi:masterfrom
xllifi:master
Open

Allow naming zip as prefix#14
xllifi wants to merge 4 commits intoRuyiLi:masterfrom
xllifi:master

Conversation

@xllifi
Copy link
Copy Markdown

@xllifi xllifi commented Sep 16, 2024

Is default behaviour. Made default here, if you want to change it.

Summary by Sourcery

Add a feature to allow users to name the zip file using a prefix through a new checkbox option in the UI. Refactor the toggleShrink function to accommodate this new feature and improve code maintainability.

New Features:

  • Introduce a new checkbox option in the UI to allow users to use a prefix as the zip file name when saving.

Enhancements:

  • Refactor the toggleShrink function to handle multiple checkbox inputs, including the new 'namezip' option.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Sep 16, 2024

Reviewer's Guide by Sourcery

This pull request introduces a new feature to allow naming the output zip file using the prefix input. It also refactors the shrink functionality to accommodate the new naming option.

File-Level Changes

Change Details Files
Added a new checkbox option to use the prefix as the zip file name
  • Added a new input element with id 'namezip'
  • Updated the save function to use the prefix as the zip file name when the new option is checked
  • Modified the toggleShrink function to handle both shrink and namezip inputs
  • Updated CSS to style the new namezip checkbox similarly to the shrink checkbox
js/main.js
index.html
style.css
Refactored the toggleShrink function to be more flexible
  • Changed toggleShrink to accept an event parameter
  • Updated the function to dynamically select elements based on the target id
  • Added a default behavior when no event is passed
js/main.js
Updated the save function and its event listener
  • Modified the save function to accept a prefix parameter
  • Updated the event listener for the download button to pass the prefix to the save function
js/main.js

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey @xllifi - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider refactoring the toggleShrink function to handle the two checkboxes separately for improved clarity and maintainability.
  • Ensure that the prefix parameter in the save function is properly passed and handled in all cases to avoid potential undefined behavior.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment thread js/main.js Outdated
@xllifi
Copy link
Copy Markdown
Author

xllifi commented Sep 16, 2024

Don't merge yet, I just found out it doesn't properly remove event from download button. Fixed in c504445

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