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

[Hideparty] Adds a toggle for each element of the UI that can be hidden #7

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChristopherJTrent
Copy link

@ChristopherJTrent ChristopherJTrent commented Apr 23, 2024

It irked me slightly that hideparty didn't support leaving the target cursor visible, so I wrote this patch to allow (at runtime) for specific UI elements to be enabled/disabled instead of all of the party frames plus the target cursor.

adds 2 commands:
/hideparty [s]how <element>
and
/hideparty [h]ide <element>

@ChristopherJTrent
Copy link
Author

I wanted to check in, since it's been quite a while, are there any particular blockers on this PR?
If there's something that would need to be changed for this to be able to be merged, please do let me know and I'll be happy to implement it.

@atom0s
Copy link
Contributor

atom0s commented Sep 29, 2024

Hello and thank you for the pull request. Sorry for the delay in response, been extremely busy with other things this last year.

Reviewing this, I cannot accept this pull in its current state as it has multiple problems.

1. Code Formatting (Whitespace)

When submitting pull requests to any code base, it is highly recommended that you follow the same formatting as the existing code. For example, your pull request is making use of tabs for whitespace while the entire rest of the addon does not. This causes the code to be poorly visible in various editors. Please make use of 4 spaces instead of tabs for whitespace.

2. Code Formatting (New Lines)

Please be mindful of your spacing between lines to follow the rest of the addon. Your new commands that have been added break the flow of the newline formatting that is already in the file to separate the different blocks of code.

3. Broken Functionality

Your changes have broken the entire original functionality of the addon, this is not ok. The addon should still work, entirely, in its original manner. This means that all of the original commands should work as they did before with your changes adding on top of those. At this time, your changes have broken the addon entirely unless people specifically use your newly added commands.

For example, load the addon and attempt to do:

/hideparty
/hideparty hide
/hideparty show

All of these are now broken with these changes.

@atom0s atom0s self-assigned this Sep 29, 2024
@atom0s atom0s added the enhancement New feature or request label Sep 29, 2024
@ChristopherJTrent
Copy link
Author

I see. I'll take another look, that functionality issue is one I didn't encounter when I was testing it, which obviously means I didn't test sufficiently. I'll work on this on my end and update this PR when I have fixed these issues. I apologize for the problems.

@ChristopherJTrent ChristopherJTrent marked this pull request as draft September 29, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants