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

[MIRROR] Fixes invisible delete button in admin painting manager. #1198

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

Steals-The-PRs
Copy link
Collaborator

Mirrored on Skyrat: Skyrat-SS13/Skyrat-tg#25720
Original PR: tgstation/tgstation#80423

Fixes #80422

This is the only Button.Confirm in the codebase that tries to wrap content tags around the component content instead of setting content="stuff" in the component's props.

image
image

I'm not sure what broke this, since it supposedly worked in the past. We've had recent PRs touching button code and confirm buttons.

But at the very least this fixes the issue and brings this unique Button.Confirm into the same standard all other Button.Confirms use for their inner content.

This hasn't been tested on a local server with DB/saved persistent paintings, however when I took other interfaces (such as the air alarm) and had their Button.Confirms wrap around their content instead of putting it in the props the content wasn't rendered (although icons were). So I'm positive this will see the button return to life.

Why It's Good For The Game

Admins can delete paintings again.

Changelog

🆑 Timberpoes
admin: Delete painting button is once again visible.
/:cl:

…B IGNORE] (#25720)

* Fixes invisible delete button in admin painting manager. (#80423)

Fixes #80422

This is the only Button.Confirm in the codebase that tries to wrap
content tags around the component content instead of setting
content="stuff" in the component's props.

![image](https://github.com/tgstation/tgstation/assets/24975989/14746233-31de-46a4-975e-87c0ce4d08b3)

![image](https://github.com/tgstation/tgstation/assets/24975989/e9361562-4e99-477b-a823-06c66f6de870)

I'm not sure what broke this, since it supposedly worked in the past.
We've had recent PRs touching button code and confirm buttons.

But at the very least this fixes the issue and brings this unique
Button.Confirm into the same standard all other Button.Confirms use for
their inner content.

This hasn't been tested on a local server with DB/saved persistent
paintings, however when I took other interfaces (such as the air alarm)
and had their Button.Confirms wrap around their content instead of
putting it in the props the content wasn't rendered (although icons
were). So I'm positive this will see the button return to life.

## Why It's Good For The Game

Admins can delete paintings again.
## Changelog
:cl:
admin: Delete painting button is once again visible.
/:cl:

* Fixes invisible delete button in admin painting manager.

---------

Co-authored-by: Timberpoes <[email protected]>
@Iajret Iajret merged commit 5ec2fca into master Dec 18, 2023
23 checks passed
@Iajret Iajret deleted the upstream-mirror-25720 branch December 18, 2023 20:52
AnywayFarus added a commit that referenced this pull request Dec 18, 2023
Iajret pushed a commit that referenced this pull request Mar 8, 2024
* Adds the colonist homestead ruin

* Adds the colonist homestead ruin

* Adds Colonist Homestead ruin

* Changes ruin name to reflect removal of RCF

* Update lavaland.dm

* Fixed error on compile

The produce bin disappeared for some reason?

* Delete lavaland_surface_RCF_homestead.dmm

* Switched produce bin to ration shelf to avoid an error

I could have sworn I already committed this. StrongDMM stopped liking the produce bin after I changed this ruin's file name to reflect the removal of the RCF.

* Final tweaks before PR

Added a cell recharger board so you can build yet another machine that drains the APC.

* TGM-ized map file

* Attempting to fix window shades

I have no idea what I'm doing. I don't know why that would cause so many checks to fail. Is it all supposed to be one word? No capital letters? Is Jupiter in retrograde?

* Replaced powered window shades with curtains

Lost my patience. No more prefab shutters, since they're somehow ruining everything.

* Added quotation marks where they were supposed to be.

See, this is why I don't program for a living. SHOULD all be fixed now?

* Adds the colonist homestead ruin

* Adds the colonist homestead ruin

* Adds Colonist Homestead ruin

* Changes ruin name to reflect removal of RCF

* Update lavaland.dm

* Fixed error on compile

The produce bin disappeared for some reason?

* Delete lavaland_surface_RCF_homestead.dmm

* Switched produce bin to ration shelf to avoid an error

I could have sworn I already committed this. StrongDMM stopped liking the produce bin after I changed this ruin's file name to reflect the removal of the RCF.

* Final tweaks before PR

Added a cell recharger board so you can build yet another machine that drains the APC.

* TGM-ized map file

* Attempting to fix window shades

I have no idea what I'm doing. I don't know why that would cause so many checks to fail. Is it all supposed to be one word? No capital letters? Is Jupiter in retrograde?

* Replaced powered window shades with curtains

Lost my patience. No more prefab shutters, since they're somehow ruining everything.

* Added quotation marks where they were supposed to be.

See, this is why I don't program for a living. SHOULD all be fixed now?

* Added Colonist Homestead in the modular areas file.

* Removes Colonist Homestead - it's in the modular_nova/code/areas lavaland file now.

* Oops, I left an extra line in there. Should be back to matching the base repo now.

* Removes Colonist homestead, moves to modular_nova

Per VinylSpiders, moved to modular_nova/modules/code/mapping/lavaland.dm

* Properly modularizes the Colonist Homestead ruin.

* Get back here you slippery quotation mark

* Removes blank a line I forgot to remove

* Renames Colonist Homestead with ruin type Lava-Ruin

* Apply suggestions from code review

---------

Co-authored-by: Bloop <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants