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(folders): Alphabetical folder order should be case insensitive. #1511

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

Conversation

ff7762
Copy link

@ff7762 ff7762 commented Nov 19, 2023

This PR is a fix for the order of folders in this issue #1286

Below in the screenshot, you can see the current functionality of this issue, which matches the fix that was described in the ticket.
image
Below is the functionality before my changes
image

You can see that the folders are now sorted in the way the ticket describes.

I have also written a test for this, to ensure maintainability of this functionality.


Questions about feature/bug bounties.
(Haven't seen another channel for correspondence.)

This is my first contribution as part of the feature bounty program. It wasn't marked as suitable for starters but I wanted to give it a shot. I hope this is useful for the users.

Is there a communications channel where payments are coordinated for accepted solutions?
Also, does high priority = Bronze tier reward? This task doesn't fit in the iron reward because that's medium and below priority.

Let me know if there are any issues, I will fix them asap.

@ff7762 ff7762 force-pushed the Alphabetical-folder-order-should-be-case-insensitive branch from 5d328b8 to d8dfbc4 Compare November 19, 2023 16:55
@ff7762 ff7762 force-pushed the Alphabetical-folder-order-should-be-case-insensitive branch from d8dfbc4 to 2e124d8 Compare November 19, 2023 17:17
Copy link
Contributor

@castaway castaway left a comment

Choose a reason for hiding this comment

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

Looks like a good fix, just need to tweak the tests to be slightly more clever, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't submit the changelog in a PR, as we generate those on deploy

Copy link
Contributor

Choose a reason for hiding this comment

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

The built-in sort() for the folder list is putting the inbox etc last, so you'll need to rethink the test

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.

3 participants