tehp / OpenPoGoWeb

Web View for OpenPoGoBot
77 stars 58 forks source link

Show current and max hp with health bar #109

Closed RolfKoenders closed 8 years ago

RolfKoenders commented 8 years ago

Hello, Its my first PR to this project and i tried to implement feature request: #107

It looks like: HP Bar

Styling could maybe be improved but it looks fine 😄

Let me know what u guys think!

Jasperrr91 commented 8 years ago

Looking good, styling could indeed be improved but so far so good.

Jasperrr91 commented 8 years ago

image

wchill commented 8 years ago

Those images look strange due to the lack of transparency.

RolfKoenders commented 8 years ago

@wchill You mean the images from @Jasperrr91? i think he's trying to show how it could be improved.

BobbyWibowo commented 8 years ago

Unexpected identifier in line 588 of main.js. Should be: 'HP: ' + pkmnHP + ' / ' + pkmnMHP + instead of the current: 'HP: ' + pkmnHP + ' / ' pkmnMHP +

RolfKoenders commented 8 years ago

@BobbyWibowo You are right. Fixed :)

BobbyWibowo commented 8 years ago

Working good on my installation. Thanks ;)

BobbyWibowo commented 8 years ago

By the way, pokemon name is missing here. Doesn't seem to be rendered in the main.js file as well. Was that intentional?

EDIT: Never mind. My mistake, missed 1 line in my copies.

wchill commented 8 years ago

I'm on mobile, can I see a render?

wchill commented 8 years ago

Of the latest commit, I should me tion

RolfKoenders commented 8 years ago

@BobbyWibowo The pokemon name is rendered here?

@wchill Would be nice to see if @BobbyWibowo can provide a screenshot.

BobbyWibowo commented 8 years ago

@RolfKoenders Yeah. My mistake. I missed 1 line when manually merging the change, lol..

@wchill Here's the preview on my desktop: render

RolfKoenders commented 8 years ago

@BobbyWibowo You're styling looks different than i had/have :hushed:

BobbyWibowo commented 8 years ago

@RolfKoenders Difference in stamina bar length is most likely caused by the fact that I viewed them on a 1080p screen. Other than that, I don't see any other differences with your first picture (other than spacing in A/D/S info) :hushed:

BobbyWibowo commented 8 years ago

@RolfKoenders Why not use margin: 0.3rem auto; for .pkmn-progress in the CSS? It guarantees that the bar is centered (regardless of the container's rendered width). squirtle

wchill commented 8 years ago

Yes, stamina bars definitely need some work. Also I'd set the max width of the bars to be up to the width of the images (120px, I think?)

BobbyWibowo commented 8 years ago

@wchill The width of the pokemon images is 120px, but limited by default to 96px by the CSS. Here's the preview of the bar with 96px max-width: squirtle_2

Jasperrr91 commented 8 years ago

@wchill not sure how the images are being rendered for you and where you're missing transparency but it's just a quick mock up of a possible style.

I think it's getting way too cluttered otherwise. A lot of clutter can be removed by hiding the words 'Candy' 'IV' A/D/S' and possibly CP

Jasperrr91 commented 8 years ago

@RolfKoenders seems like the bar isn't completely in line with the text. Probably some padding somewhere?

image

BobbyWibowo commented 8 years ago

@Jasperrr91 The extra 'spacing' appears to be caused by these lines: .container .row { margin-left: -0.75rem; margin-right: -0.75rem; } in materialize.min.css. Setting them back to auto removes the spacing issue.

RolfKoenders commented 8 years ago

Alright! Just got home! Will take a look at the feedback! What shall we do?

EDIT: I had to do some coding before going to bed. Here is what i have now. Made the height a bit smaller and made sure its centered. I also really liked the 96px width as @BobbyWibowo showed.

Current state after my next commit:

health bar

BobbyWibowo commented 8 years ago

Here is my latest edit. I won't make any pull request since my local copies are heavily modified here and there, and I only merged changes that I actually needed from the pull requests, so yeah, it's a pain just thinking about it (also, I'm using PokemonGoF instead of OpenPoGoBot, so my main.js doesn't have the whole websocket thingy). Anyway if you like any of the ideas from my screenshot, feel free to implement it :wink: pokego-bot-pokemon-list Hint about the blue glow: I read somewhere that the blue glows in the PokeGO app means that the Pokemon was captured since 24 hours ago or less. So yeah. I thought, might as well.

Jasperrr91 commented 8 years ago

Very nice @BobbyWibowo, can you post the modified HTML/CSS?

RolfKoenders commented 8 years ago

@BobbyWibowo Looks good! I will take a look and make some similar changes!. I like the bold text! (hp, cp etc..).

BobbyWibowo commented 8 years ago

@Jasperrr91 @RolfKoenders Here are the pastebins of the whole main.js and main.css file. You'll have to salvage whichever parts you want by yourself, if you want, lol main.js: http://pastebin.com/Ws3G6zw1 (uses Moment.js for the relative time display) main.css: http://pastebin.com/gi5vneVa (custom changes were only done at the bottom of the css) Psst. it also includes fix for experience progress bar: https://github.com/OpenPoGo/OpenPoGoWeb/issues/83#issuecomment-236033478

UPDATE: Here's the git repo of my version: https://github.com/BobbyWibowo/OpenPoGoWeb. I had integrated all my changes to wchill/refactor branch.

Reaver01 commented 8 years ago

It looks like you guys are still working on this. Let me know when it should be approved and merged.

RolfKoenders commented 8 years ago

@Reaver01 I will look at the changes @BobbyWibowo made, there are some nice changes. I will try to make a final commit with this which we can improve on. Then at least we have a decent version. Later on improvements can be made. Will look at this in a few hours, will be ready tonight! @Reaver01 @BobbyWibowo Appreciate the work man!

Reaver01 commented 8 years ago

@RolfKoenders sounds good. also keep in mind the wchill/refactor branch as that branch is making a lot of changes and is what we are looking at for long term. Maybe see if you can write this to stick into that branch as well.

RolfKoenders commented 8 years ago

@Reaver01 Cool, wasn't aware. Will take that into account!

RolfKoenders commented 8 years ago

@BobbyWibowo I took the changes u made for the healthbar and the stats. (Bold text, blue iv text). I think the other changes like 'time ago' can be a separate PR.

@Reaver01 Let me know if u like it in this way!

Reaver01 commented 8 years ago

👍

Approved with PullApprove

BobbyWibowo commented 8 years ago

My latest edit shows the HP text within the HP bar now. I had also made capture time to only appear while sorting by time. Making it much cleaner, imho. pokego-bot-pokemon-list_3 If you're interested to see all the changes, I've forked the repo and commited all my changes there: https://github.com/BobbyWibowo/OpenPoGoWeb. FYI, I made the changes on wchill/refactor branch. Also, due to the fact that it's quite heavily modified, I'm not gonna do any PR. So yeah, as always, feel free to salvage whatever you need.

MaxLeiter commented 8 years ago

:+1: thanks!

Approved with PullApprove