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

Feature: Sacks page in pv #891

Merged
merged 9 commits into from
Nov 4, 2023
Merged

Conversation

CalMWolfs
Copy link
Contributor

@CalMWolfs CalMWolfs commented Oct 22, 2023

From testing did not crash with missing repo file
Shows an overview of all sacks along with the breakdown of individual sacks
Rune sack works differently where it only shows the runes in the player's sack API whereas everything else shows the whole sack regardless of how many items you have in the sack.
Is part of the inventories page

NopoTheGamer
NopoTheGamer previously approved these changes Oct 25, 2023
Copy link
Collaborator

@jani270 jani270 left a comment

Choose a reason for hiding this comment

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

You might want to move the "Back" button within sack to the bottom left or right and then moving the arrows down a bit. 
Another thing is maybe changing the amount of items to light gray instead of green? Not sure if it will look good, tho.
The last thing i saw is that maybe you should add a small note in the sack lore that you can click on them to see a more detailed view of the sack. 

@CalMWolfs
Copy link
Contributor Author

You might want to move the "Back" button within sack to the bottom left or right and then moving the arrows down a bit.  Another thing is maybe changing the amount of items to light gray instead of green? Not sure if it will look good, tho. The last thing i saw is that maybe you should add a small note in the sack lore that you can click on them to see a more detailed view of the sack.

Did all of these, light grey does look good

@jani270 jani270 requested a review from nea89o November 2, 2023 11:52
@jani270
Copy link
Collaborator

jani270 commented Nov 2, 2023

Tested out the new changes and they look nice. I have 3 suggestions you could think about if you want to do them (i have no idea if they will look good.

  1. Make the Click for more details text yellow
  2. Make the Total Value Gold (just the value not the text)
  3. Make Item Stored in light green §a (also just the number not the text)

@Lulonaut Lulonaut merged commit f197b51 into NotEnoughUpdates:master Nov 4, 2023
5 checks passed
@CalMWolfs CalMWolfs deleted the sacks_pv branch November 4, 2023 14:43
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.

5 participants