Lua UI improvements #68887

Y0SH1M4S73R · 2022-07-31T03:14:58Z

About The Pull Request

Atomized from #68816, with a little addition. Fixes some dumb formatting issues with the lua editor, adds a "jump to bottom" button when viewing the state log, and paginates the state logs.

Why It's Good For The Game

UI fixes and qol are good.

Changelog

🆑
fix: Fixes the formatting of the admin lua editor
qol: In the admin lua editor, there will be a "jump to bottom" button in the log viewer if the log is longer than can be shown in the window.
qol: The lua editor log viewer is now split into pages of at most 50 log entries.
/:cl:

padding is surprisingly useful to deal with stupid tgui jank

Watermelon914 · 2022-07-31T07:11:34Z

tgui/packages/tgui/interfaces/LuaEditor/index.js

+    const jumpButtonCondition =
+      activeTab === 'log' &&
+      this.sectionRef.current?.scrollableRef.current.scrollHeight >
+        this.sectionRef.current?.scrollableRef.current.clientHeight;


you should hook onto onScroll instead and make this a state var, otherwise it'll fail to update if you scroll up

Watermelon914 · 2022-07-31T07:12:12Z

tgui/packages/tgui/interfaces/LuaEditor/index.js

+    super(props);
+    this.sectionRef = createRef();
+
+    window.addEventListener('resize', () => this.setState({}));


use this.forceUpdate() instead

Y0SH1M4S73R · 2022-08-01T00:17:00Z

still gotta do log pagination, DNM until next commit

Y0SH1M4S73R · 2022-08-01T05:24:33Z

drafting because I wanna do some more work on this tomorrow

Y0SH1M4S73R · 2022-08-01T05:39:53Z

eh I decided to make those changes anyways

Watermelon914 · 2022-08-02T10:22:50Z

tgui/packages/tgui/interfaces/LuaEditor/index.js

+    const [modal, setModal] = useLocalState(
+      this.context,
+      'modal',
+      noStateYet ? 'states' : null
+    );
+    const [activeTab, setActiveTab] = useLocalState(
+      this.context,
+      'activeTab',
+      showGlobalTable ? 'globals' : 'tasks'
+    );
+    const [input, setInput] = useLocalState(this.context, 'scriptInput', '');
+    const [shouldUpdateScroll, setShouldUpdateScroll] = useLocalState(
+      this.context,
+      'shouldUpdateScroll',
+      false
+    );


no reason for local state if you have a class object.
You can just store the state directly on the class

for some of these local state variables, yes, but others are used in more than one function component.
shouldUpdateScroll is also set from DM code using the tgui_shared_states var.

stylemistake · 2022-08-02T19:53:45Z

code/modules/admin/verbs/lua/lua_editor.dm

 			current_state.get_globals()
 			data["globals"] = kvpify_list(refify_list(current_state.globals))
 	data["states"] = SSlua.states
 	data["callArguments"] = kvpify_list(refify_list(arguments))
+	LAZYSET(tgui_shared_states, "shouldUpdateScroll", "true")


You can not use this private tgui API. Expose the variable via ui_data instead if you need this much control.

i was in the process of doing this as you were making this comment, so good to know I shouldn't use it.

stylemistake · 2022-08-03T07:32:16Z

tgui/packages/tgui/interfaces/LuaEditor/index.js

    if (shouldUpdateScroll) {
-      setShouldUpdateScroll(false);
+      shouldUpdateScroll = 0;
      setTimeout(this.handleSectionScroll, 0);
    }


I think the way of dealing with scroll is way too overcomplicated, since you can write this entire effect in componentDidMount() to run once without dependency on any external dm data or extra re-renders.

componentDidMount() { setTimeout(this.handleSectionScroll, 0); }

Also setting it to 0 achieves nothing, because next UI update sends TRUE again. Refactor this once more.

I didn't quite get the behavior I was looking for putting the code in componentDidMount, but I did in componentDidUpdate

stylemistake · 2022-08-03T07:36:36Z

tgui/packages/tgui/interfaces/LuaEditor/index.js

+    if (forceModal || forceViewChunk) {
+      const [newModal, newViewChunk] = [forceModal, forceViewChunk];
+      forceModal = null;
+      forceViewChunk = null;
+      setModal(newModal);
+      setViewedChunk(newViewChunk);
+    }


Another thing you probably haven't realized is that you can insert a default value into useLocalState, and that default value can be forceModal. And this entire chunk of code goes away.

Better yet, since you're using a class-based component, you can use this.state instead for better performance, and again, implement this effect in componentDidMount to run only once.

componentDidMount() { if (props.forceModal) { this.setState({ showModal: true }) } }

In general, setting state inside render() is forbidden because it is inefficient, can cause endless update cycles and there are better ways of doing it. In modern React, this would've been a compilation error.

in the context of this PR, modal is used both by the class-based root component and several function-based child components. Would you suggest I do something different, such as making all the child components take a function to be called when clicking the "close" button, and give all those child components a function that includes the this.setState for the root component?

modal is used both by the class-based root component and several function-based child components

I guess this is OK.

Overall readability has improved.

github-actions · 2022-08-12T00:18:06Z

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

* tgui input hotfix tgstation/tgstation#64698 * changes how tgui handles static data tgstation/tgstation#64757 * tgstation/tgstation#64885 * tgstation/tgstation#65008 * cooldown check * read preferences doenst exist here * tgstation/tgstation#65378 * Adds support for embedding react components in tgui chat * tgstation/tgstation#65686 * tgstation/tgstation#65943 * previous part 2 * oops forgot * tgstation/tgstation#66220 * tgstation/tgstation#66299 * FUCk forgot 1 * tgstation/tgstation#59914 * tgstation/tgstation#66304 * tgstation/tgstation#66309 * tgstation/tgstation#66317 * tgstation/tgstation#66691 * tgstation/tgstation#66757 * small changes from 66933 * tgstation/tgstation#67137 * tgui part of tgstation/tgstation#67691 and tgstation/tgstation#66971 * forgot public html * tgstation/tgstation#67935 prewritepackages * after write packages * tgstation/tgstation#67937 * oops * tgstation/tgstation#68216 * tgstation/tgstation#68356 * tgstation/tgstation#68679 * tgstation/tgstation#68887 * prettier ignore outside * tgstation/tgstation#69219 * tgstation/tgstation#69459 * tgstation/tgstation#69527 * tgstation/tgstation#69735 * tgstation/tgstation#69786 * tgstation/tgstation#70779 * boop * Update code/modules/mob/living/silicon/ai/examine.dm * fix ? * io * yo * prettier * prettier + other stuff + tgui say etc * okay ? * webpack tguisay * ntoswindow * tguialert move * tgui say work ? * tgstation/tgstation#71037 * rollback tguisay * comment out tgui say * remove tguisay dm files * updates tgui folder * whytflinter * rerun * fixes stack trace on fullupdate * woops, removes \n from join() * balloon alert * timeleft shenanigan solve stack_trace * check if fix linter * add it back * woo linter

Y0SH1M4S73R added 3 commits July 30, 2022 17:03

fixes the formatting of the lua editor ui

86f88dc

padding is surprisingly useful to deal with stupid tgui jank

Jump-To-Bottom button for lua editor

b64fa30

jump button only appears when the log can't fit in the window

ca22a65

tgstation-server added Fix Rewrites a bug so it appears in different circumstances Quality of Life Increasing esword damage is not a quality of life for traitors UI We make the game less playable, but with round edges labels Jul 31, 2022

github-actions bot requested a review from stylemistake July 31, 2022 03:16

github-actions bot added the Merge Conflict Adding upstream files to your repo via drag and drop won't resolve conflicts label Jul 31, 2022

Watermelon914 requested changes Jul 31, 2022

View reviewed changes

Y0SH1M4S73R added 3 commits July 31, 2022 20:13

modals properly scale with the window

8037257

log jump button improvements

a2b8b9c

Merge remote-tracking branch 'upstream/master' into lua_ui_improvements

e51c9c4

tgstation-server removed the Merge Conflict Adding upstream files to your repo via drag and drop won't resolve conflicts label Aug 1, 2022

log pagination

b96ed38

Y0SH1M4S73R marked this pull request as draft August 1, 2022 05:24

small pagination improvements

5ba9ce1

Y0SH1M4S73R marked this pull request as ready for review August 1, 2022 05:39

Y0SH1M4S73R requested a review from Watermelon914 August 2, 2022 01:50

Watermelon914 requested changes Aug 2, 2022

View reviewed changes

stylemistake reviewed Aug 2, 2022

View reviewed changes

addresses review items

04aaddb

Y0SH1M4S73R requested review from Watermelon914 and stylemistake August 2, 2022 20:19

stylemistake reviewed Aug 3, 2022

View reviewed changes

further refactoring

9fc84b6

github-actions bot requested a review from stylemistake August 3, 2022 20:38

github-actions bot added the Stale Even the uncaring universe rejects you, why even go on label Aug 12, 2022

Watermelon914 merged commit 6cc161e into tgstation:master Aug 12, 2022

tgstation-server added a commit that referenced this pull request Aug 12, 2022

Automatic changelog generation for PR #68887 [ci skip]

0a38a25

github-actions bot added a commit that referenced this pull request Aug 12, 2022

Updating GBP balances for #68887

607cb4f

TaleStationBot mentioned this pull request Aug 12, 2022

[MIRROR] [MDB IGNORE] Lua UI improvements TaleStation/TaleStation#1974

Merged

SkyratBot mentioned this pull request Aug 12, 2022

[MIRROR] Lua UI improvements [MDB IGNORE] Skyrat-SS13/Skyrat-tg#15561

Merged

Y0SH1M4S73R mentioned this pull request Aug 24, 2022

Syncs lua scripting code with tgstation StarbloomSS13/StarbloomSS13#229

Merged

Y0SH1M4S73R deleted the lua_ui_improvements branch October 15, 2022 01:48

SplinterGP added a commit to SplinterGP/TerraGov-Marine-Corps that referenced this pull request Nov 4, 2022

https://github.com/tgstation/tgstation/pull/68887

b2d0583

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lua UI improvements #68887

Lua UI improvements #68887

Y0SH1M4S73R commented Jul 31, 2022 •

edited

Loading

Watermelon914 Jul 31, 2022 •

edited

Loading

Watermelon914 Jul 31, 2022

Y0SH1M4S73R commented Aug 1, 2022 •

edited

Loading

Y0SH1M4S73R commented Aug 1, 2022

Y0SH1M4S73R commented Aug 1, 2022

Watermelon914 Aug 2, 2022

Y0SH1M4S73R Aug 2, 2022 •

edited

Loading

stylemistake Aug 2, 2022

Y0SH1M4S73R Aug 2, 2022

stylemistake Aug 3, 2022 •

edited

Loading

Y0SH1M4S73R Aug 3, 2022

stylemistake Aug 3, 2022 •

edited

Loading

Y0SH1M4S73R Aug 3, 2022

stylemistake Aug 4, 2022 •

edited

Loading

github-actions bot commented Aug 12, 2022

Lua UI improvements #68887

Lua UI improvements #68887

Conversation

Y0SH1M4S73R commented Jul 31, 2022 • edited Loading

About The Pull Request

Why It's Good For The Game

Changelog

Watermelon914 Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Watermelon914 Jul 31, 2022

Choose a reason for hiding this comment

Y0SH1M4S73R commented Aug 1, 2022 • edited Loading

Y0SH1M4S73R commented Aug 1, 2022

Y0SH1M4S73R commented Aug 1, 2022

Watermelon914 Aug 2, 2022

Choose a reason for hiding this comment

Y0SH1M4S73R Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

stylemistake Aug 2, 2022

Choose a reason for hiding this comment

Y0SH1M4S73R Aug 2, 2022

Choose a reason for hiding this comment

stylemistake Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

Y0SH1M4S73R Aug 3, 2022

Choose a reason for hiding this comment

stylemistake Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

Y0SH1M4S73R Aug 3, 2022

Choose a reason for hiding this comment

stylemistake Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 12, 2022

Y0SH1M4S73R commented Jul 31, 2022 •

edited

Loading

Watermelon914 Jul 31, 2022 •

edited

Loading

Y0SH1M4S73R commented Aug 1, 2022 •

edited

Loading

Y0SH1M4S73R Aug 2, 2022 •

edited

Loading

stylemistake Aug 3, 2022 •

edited

Loading

stylemistake Aug 3, 2022 •

edited

Loading

stylemistake Aug 4, 2022 •

edited

Loading