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

Fixed: As Many UI Issues as I can Handle #1622

Merged
merged 37 commits into from
Jun 24, 2024
Merged

Fixed: As Many UI Issues as I can Handle #1622

merged 37 commits into from
Jun 24, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Jun 13, 2024

Summary

This PR fixes as many possible discrepancies between the App UI and the Figma design as possible.

Methodology is as follows:

  • Set the App window to 70% opacity.
  • Set the Figma project to 100% size. (Ctrl+0)
  • Overlay the App Window ontop of the Figma

If the App Window does not match Figma, figure out what's wrong with the App.

In this PR I've basically went over the entire UI of the App, and made almost everything match 1:1 pixel by pixel with Figma.

Commit / Change List

2024 Jun 20:

Fixes

fixes #1551
fixes #1658 (to some degree)

And a bunch of other non-reported tickets.

Sewer56 added 27 commits June 13, 2024 04:37
Buttons had excessive padding on left and right
    - Reduced from 7 to 4, now icon buttons are round, and design is more closely matched
    - Example: Icon in Pill now Makes Rounded Circle, instead of a thick rectangle
Icons in buttons were a few pixels too small.
    - They were 20px but should have been 24px.
    - Example: AdvancedInstaller, My Games
Help Icon in Top Bar now Matches Sizes of Other Icons
Description Icon was wrong size in FOMOD Installer
Now is sized correctly and buttons don't have unnecessary margin
This icon is used in FileTreeView and Nowhere else. So the entry was modified directly to match the current icon in `FileTreeView`
@Sewer56 Sewer56 added the Design UI/UX This is related to the UI. label Jun 13, 2024
@Sewer56 Sewer56 self-assigned this Jun 13, 2024
@erri120
Copy link
Member

erri120 commented Jun 13, 2024

Did you rebase already? Some of these fixes might already been in main.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 13, 2024

Some of these fixes might already been in main.

Only renaming Health check to Health Check.
It happened to be community PR'd as I was working on this branch.

Note: This branch was started around midnight today when I woke up.
And everything was being merged from main as main was being updated.
Aside from the above exception, everything here is new.

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

Tab headers icon sizes are changing when selected, which is likely due to changed default icon size.

Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Jun 17, 2024
@Sewer56
Copy link
Member Author

Sewer56 commented Jun 17, 2024

Please note, phrasing like

Changing this default has caused a bunch of icons over the app to now not have correct size.

Needs further explanation.
Did UI things change on main?
Which icons are we talking about? etc.

I spent the whole day last Thursday frantically overlaying the App window over the Designs, to ensure each icon lines up.

Design got to see a bunch of this too, in a huddle.

I wound up doing that for just about every view in the App, so if I've missed some cases e.g. 'X is selected', those cases need to be stated exactly on the feedback so I am aware what needs fixing.


Misc Note: Design decided to update LeftMenu icons to the size in this PR, but because the decision was done late Thursday, it probably hasn't been reflected in actual design files.

@Al12rs
Copy link
Contributor

Al12rs commented Jun 18, 2024

The issues we found in review huddle with Design were:

  • Selected tab headers have different icon sizes from non selected ones
  • GameWidget icons were inconsistent (pushed a commit to fix this)
  • Cog and Help icon in TopBar were too big
  • All FileTrees had increased sizes now, not sure if this actually matched designs but we might have decided to use smaller size at the time instead since it was more compact.
  • Icons in various buttons were now different in size, e.g. advanced installer, GameWidget etc.

Given the various inconsistencies, one would have to go through the entire app again to check each icon.
Changing the default size like that is not advised because it requires going though the entire app, both for the implementer and the reviewer, which takes a lot of time.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 19, 2024

Changing the default size like that is not advised because it requires going though the entire app,

@Al12rs @captainsandypants So what should I do about this?

When doing this, I did in fact go through every page of the App and overlay them over the design.

If what you say is true, for example, going with smaller icons in file tree(s) being a deliberate choice, then what should I use as the source of truth?

Do I need to take @captainsandypants and go through every screen again? But in huddle

Icons in various buttons were now different in size, e.g. advanced installer, GameWidget etc.

Because these, for example, were in fact matching the designs on Figma. And I even asked specifically about GameWidget since those had a different size.

Copy link
Contributor

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Jun 20, 2024
@Al12rs
Copy link
Contributor

Al12rs commented Jun 20, 2024

Reviewing again and marking things that need fixing, not necessarily in this PR:

  • GameWidged icon inconsistencies:
    image
  • Tab headers icon size seems very big. Alignment definitely needs fixing:
    image
  • some icons don't change color correctly:
    image
  • Menu bar text and icon don't seem aligned:
    image
  • TopBar settings and help icons seem too big:
    image

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 20, 2024

@Al12rs we'll probably need to do a review of the design in general with @captainsandypants at one point, and write up a list of things that need standardized. I was about to ask them for a huddle.

I noticed that even within the design, there were places where common icons had different sizes between the different designs.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 20, 2024

some icons don't change color correctly:

This one actually has to do with how the icon is loaded. IIRC I actually need to convert the SVGs into a Sized ViewBox with an internal Geometry (kinda like how Projectanker does). I let Lausandy know about it last week ahead of time.

It'll be a bit more involved, so this is best left for a follow-up PR.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 20, 2024

I opened

For the icon colouring. Shouldn't be too hard, I already did the research.

@Sewer56 Sewer56 merged commit 5f67d5c into main Jun 24, 2024
11 checks passed
@erri120 erri120 deleted the fixup-icons branch July 1, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design UI/UX This is related to the UI.
Projects
Archived in project
3 participants