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

18ZOO - Update StockMarket to show bigger cells when there is any user info in any cell #9731

Merged
merged 1 commit into from
Oct 14, 2023

Conversation

carlorusso1984
Copy link
Contributor

Fixes #9693

Before clicking "Create"

  • Branch is derived from the latest master
  • Add the pins label if this change will break existing games
  • Code passes linter with docker compose exec rack rubocop -a
  • Tests pass cleanly with docker compose exec rack rake

Implementation Notes

  • Explanation of Change
    As requested inside the issue, the stock market was updated to support the info, but only for the 2d grid

  • Screenshots

image
  • Any Assumptions / Hacks

@@ -69,7 +69,7 @@ class StockMarket < Snabberb::Component
textAlign: 'center',
position: 'absolute',
bottom: "#{PAD}px",
width: "#{WIDTH_TOTAL - (2 * PAD) - (2 * BORDER)}px",
width: "#{WIDTH_TOTAL + PRICE_HEIGHT - (2 * PAD) - (2 * BORDER)}px",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added PRICE_HEIGHT to both width and height to keep the cells as square

@tobymao tobymao merged commit 1c40ce5 into tobymao:master Oct 14, 2023
1 check passed
@tobymao
Copy link
Owner

tobymao commented Oct 14, 2023

i reverted this, does this change the default stock market look?

@carlorusso1984
Copy link
Contributor Author

It should change only the stock market when used as 2D and there are userInfo

@serpentium
Copy link

i reverted this, does this change the default stock market look?

Can you check again? Carlo answer it it is only here.

@carlorusso1984
Copy link
Contributor Author

@serpentium I have done this new PR to avoid changing default stock market: #9790
I think I'll have that merged this weekend

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.

18zoo market details
3 participants