search

ccomeaux / boardgamegeek4android

BoardGameGeek application for Android (unofficial)
GNU General Public License v3.0
228 stars 47 forks source link

Add ability to specify negative scores #83

Closed cincodenada closed 6 years ago

cincodenada commented 7 years ago

Many games allow negative scores, and as the website seems to support them just fine, I see no reason the BGG app shouldn't allow entry of negatives.

The UI is certainly open to suggestions, I decided that instead of messing with the buttons I'd just add a +/- button to mirror the delete button, as that seemed sensical and intuitive. I also considered adding a "-" button and bumping the checkbox button down to a wide button on its own row, but figured this was less intrusive.

In doing so I also fiddled with the weights a bit and moved the left padding to the +/- button, as is logical.

I think I covered all the details in the affected helpers. I also added maybeUpdateOutput(), which encompasses a bit of code that was already duplicated twice and would have had to been a third time to handle the negative button.

ccomeaux commented 7 years ago

Thanks for the PR. I'll look over it and give you some feedback soon. Of note, you CAN enter a negative score, but you need to go to the player edit screen where you have full control of the score, even adding characters.

cincodenada commented 7 years ago

Aha! I didn't know that, thanks for the pointer. This just makes it easier to enter negative scores then ☺️

Sent from my Motorola XT1635-02 using FastHub

kamal-kamalmohamed commented 6 years ago

@ccomeaux, @cincodenada: Hey there! I've just checked out the repo and looking for a way to contribute. This PR has been open for a while. I'd be happy to review it to help merge it in. Or was it abandoned for a reason? 😄

ccomeaux commented 6 years ago

There's no good reason it was abandoned. If I remember correctly, at the time I was thinking there was a better place for the "-" button, but I don't know what that was. I'll try to check out the branch tonight to play with it. If it looks good I will merge it.

cincodenada commented 6 years ago

I agree - it is a bit awkward, but it was also the least awkward I could figure, so 🤷‍♂️

Also, I've been running a version with this patch ever since I submitted the PR, and while I haven't entered that many negative sores, it's worked when I have and hasn't otherwise had problems.

I'll take another pass over it as well when I get a chance this week.

On Mon, Jun 4, 2018, 04:49 Chris Comeaux notifications@github.com wrote:

There's no good reason it was abandoned. If I remember correctly, at the time I was thinking there was a better place for the "-" button, but I don't know what that was. I'll try to check out the branch tonight to play with it. If it looks good I will merge it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ccomeaux/boardgamegeek4android/pull/83#issuecomment-394327400, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdR43pbTjLgvb0uWr3yzh09LMJV7iXzks5t5R60gaJpZM4OeqaZ .