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

TGUI ID Computer #14766

Merged
merged 19 commits into from Aug 4, 2023
Merged

TGUI ID Computer #14766

merged 19 commits into from Aug 4, 2023

Conversation

Garash2k
Copy link
Contributor

@Garash2k Garash2k commented Jun 29, 2023

About the PR

Rising #11970 from it's grave from when I worked on it 6 months ago. Adressed the "loading from game data point," made it compatible with dept. ID computers, and added a few goodies such as a selection on the card look button, faster updating card name, and some put_in_hand_or_eject.
I know that compactness is a concern when going from raw html to tgui, but I think this result is ok, with every job being visible, and the area accesses peeking out of the fold with this new default window size.

image

older pics:

image
image
image
image
image

Why's this needed?

Color coding and groupings should make it easier to use. Also a useless Login feature has been removed, making it faster to use. Plus, TGUI looks more modern than raw html

Changelog

(u)Garash
(+)New snappier UI for the Identification Computers, go commit ID fraud today!

[UI][Feature]

@keywordlabeler keywordlabeler bot added A-UI Modifies UI in some way. Automatically applied on a change to tgui/ C-Feature A new feature or enhancements to existing features labels Jun 29, 2023
@github-actions github-actions bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 29, 2023
@Lynncubus
Copy link
Contributor

A little nitpick, but the background colors make the interface look extremely busy and the options hard to read.

@Glamurio
Copy link
Contributor

Love this, great job, fantastic work, this makes me excited to work on my HoP rework again.

A little nitpick, but the background colors make the interface look extremely busy and the options hard to read.

Suggestion: Only have the headers colored, but the main content be black. Here's a mockup:

image

@Lynncubus
Copy link
Contributor

Love this, great job, fantastic work, this makes me excited to work on my HoP rework again.

A little nitpick, but the background colors make the interface look extremely busy and the options hard to read.

Suggestion: Only have the headers colored, but the main content be black. Here's a mockup:

image

Yeah, something like that, I'd suggest making them collapsible, see the WeaponVendor as an example. And don't darken the colors by 20% as they ruin the contrast and makes the colors hideous in my opinion

@Garash2k
Copy link
Contributor Author

Thinking about ui_static_data,
standard_jobs could easily be in it.
accesses_by_area would have to have it's allowed indicator moved somewhere else, we could calculate it in js by passing modify.access in ui_data
I'll do that when I get home along with the suggested box tweaks

@Garash2k
Copy link
Contributor Author

Yeah, something like that, I'd suggest making them collapsible, see the WeaponVendor as an example. And don't darken the colors by 20% as they ruin the contrast and makes the colors hideous in my opinion

The reason the colors are darkened is for accessibility concerns. Because the text is white, it has poor readability unless the background color is darkened
image
image

As for collapsible, I'm not seeing the use for it, each section would only collapse 1 to 3 lines of buttons, and it's not like a seed vendor where you can be interested in a specific category that would otherwise be a few pages down.

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Jun 30, 2023
@Glamurio
Copy link
Contributor

The reason the colors are darkened is for accessibility concerns. Because the text is white, it has poor readability unless the background color is darkened.

Totally valid, however they have a point about the colors. Whenever it comes to accessibility, I like to do a luminance comparison of the colors and decide if using black font color might be better. I'm not sure however, how good it would look to have one or two departments with black headers.

Perhaps we could achieve a happy medium? How does 10% look, does that still pass or at least barely fail?

Additionally, since we are using HTML and presumably CSS, you could try the text-shadow property:
text-shadow: 0 0 1px black, 0 0 1px black, 0 0 1px black, 0 0 1px black;
With this, white text could be outlined black. That way, the text will stand out no matter the background.

@Glamurio
Copy link
Contributor

Glamurio commented Jul 6, 2023

image

Small typo, you misspelled "maintenance"

@github-actions
Copy link
Contributor

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label Jul 22, 2023
@Garash2k
Copy link
Contributor Author

!merge_upstream

@github-actions
Copy link
Contributor

@flappybatpal flappybatpal removed the S-Stale An inactive PR that has had no updates in the past two weeks label Jul 22, 2023
tgui/packages/tgui/styles/interfaces/IDComputer.scss Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/IDComputer.js Outdated Show resolved Hide resolved
Co-authored-by: ZeWaka <zewakagamer@gmail.com>
@ZeWaka ZeWaka added the C-Rework Reworks a feature label Jul 22, 2023
Copy link
Member

@ZeWaka ZeWaka left a comment

Choose a reason for hiding this comment

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

actually one small change i see that can help with the verticality
do this with the top - probably a 2/3 1/3 split because long names
image

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Jul 23, 2023
@Garash2k
Copy link
Contributor Author

Horizontal!
image

Also while testing it, I noticed that very long buttons would grow out of the window/their parent. With a whitespace property we can make them wrap, which is better than then ending up on top of the Pronouns/PIN section.
image

tgui/packages/tgui/interfaces/IDComputer.js Outdated Show resolved Hide resolved
@Garash2k
Copy link
Contributor Author

!merge_upstream

@github-actions
Copy link
Contributor

@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Jul 24, 2023
@ZeWaka ZeWaka added the S-Testmerged [Dev Only] Testmerged for extended testing (applied by bot) label Aug 1, 2023
@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Aug 1, 2023
@github-actions github-actions bot added S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict and removed S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict labels Aug 2, 2023
@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Aug 2, 2023
@ZeWaka ZeWaka removed the S-Testmerged [Dev Only] Testmerged for extended testing (applied by bot) label Aug 4, 2023
@ZeWaka ZeWaka merged commit 11ba914 into goonstation:master Aug 4, 2023
22 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Modifies UI in some way. Automatically applied on a change to tgui/ C-Feature A new feature or enhancements to existing features C-Rework Reworks a feature size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants