General improvements of the styling

eclipse-cdt-cloud / vscode-memory-inspector

vscode memory inspector

https://open-vsx.org/extension/eclipse-cdt/memory-inspector

Eclipse Public License 2.0

6 stars 10 forks source link

General improvements of the styling #130

Closed planger closed 4 months ago

planger commented 4 months ago

What it does

Properly aligning the icons in the memory widget
Aligning and line breaking in the address / offset / length area
Better spacing and coloring in the advanced options overlay
Adding own scrollbar in the advanced options overlay
Avoid line breaking in the label column of the hover

How to test

Test how you feel the layouting looks in the upper part of the memory inspector and the options overlay. Try different themes too. Also try resizing the view to observe how the flex components break into new lines.

Review checklist

[X] as an author, I have thoroughly tested my changes and carefully followed the contribution guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the code of conduct

jreineckearm commented 4 months ago

Changes look good!

Two findings which I can confirm to have been there before your changes:

If you have reduce height of the window while the advanced options pop-up is open, then the pop-up jumps upwards and overlays the options button.
The pop-up overlays the main window scrollbar. Probably not super-problematic as the expectation would be that the advanced option dialog is not present when navigating through the data.

planger commented 4 months ago

Thank you for your feedback! I've briefly looked into the two findings.

If you have reduce height of the window while the advanced options pop-up is open, then the pop-up jumps upwards and overlays the options button.

The pop-up overlays the main window scrollbar. Probably not super-problematic as the expectation would be that the advanced option dialog is not present when navigating through the data.

I'm afraid, primereact dynamically sets the top and left style attribute in the element style directly. I guess we could customize this dynamic behavior, but it isn't quite as straightforward as just setting a value, as far as I could determine in the docs. So I'm not sure it is worth pursuing this. What do you think?

jreineckearm commented 4 months ago

So I'm not sure it is worth pursuing this. What do you think?

Thanks for investigating! I agree that now probably isn't the right time for it. Let's see if real users come back with feedback about this.