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

Issue735/bailfilter #815

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Conversation

ibixina
Copy link
Contributor

@ibixina ibixina commented Nov 5, 2024

Bail cost for jail filter #735

added a bail cost filter in the jail filter
modified the existing code in ttJailFilter.js to have a new textbox for bail cost filter.

image

@ibixina
Copy link
Contributor Author

ibixina commented Nov 5, 2024

This pull request fixes #735 .

Copy link
Collaborator

@DeKleineKobini DeKleineKobini left a comment

Choose a reason for hiding this comment

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

From your screenshot, it seems so packed with this added, so I'm requesting a review from the other collaborator as well.

As mentioned in the contributing guide, you are missing the changelog entry and you aren't in the team.js file yet either.

extension/scripts/features/jail-filter/ttJailFilter.js Outdated Show resolved Hide resolved
extension/scripts/features/jail-filter/ttJailFilter.js Outdated Show resolved Hide resolved
…bail filter + name updated timeMatch -> timeText
@ibixina
Copy link
Contributor Author

ibixina commented Nov 5, 2024

do you think it might be better if it was under the faction filter?

@Sashank999
Copy link
Collaborator

do you think it might be better if it was under the faction filter?

Rather than grouping up filters, I have added code for making the filters wrap. Currently, only Auction House Filter and this new Jail Filter will be affected by this change.

Copy link
Collaborator

@Sashank999 Sashank999 left a comment

Choose a reason for hiding this comment

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

That is the change that I suggest. Everything else is good to go.

extension/scripts/features/jail-filter/ttJailFilter.js Outdated Show resolved Hide resolved
@Sashank999 Sashank999 merged commit b5b8565 into Mephiles:master Nov 7, 2024
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