Skip to content

Conversation

nckrtl
Copy link
Contributor

@nckrtl nckrtl commented Apr 4, 2024

Was taking another look at the toolbar and found some more things that could be improved

  • Different paddings were being used for left and right side, this is now consistent
  • Weird hover state on active tab/element
  • Text appears bit fuzzy because of text shadow, I think its more clear to show it without a text shadow
  • Icons on the right side seem to have different paddings, made those consistent as well
  • applied better vertical alignment for the icons used on the right side

Visual side by side:
visual-changes

Also noticed that the rows of the request data is not nicely aligned anymore:
Screenshot 2024-04-04 at 21 39 23
I saw this remark: #1605 (comment) and also the PR that is causing the misalignment: #1608

This CSS fixes the alignment, while making sure there is enough space for the key to be fully displayed and not being cut off.

dl.phpdebugbar-widgets-kvlist dt {
    width: 25%;
    width: calc(25% - 10px);
}

Note that this is now properly calculating the width instead of width: calc(25%-10px); which I included and then removed in some commits ago as it was invalid due to missing spaces.

Screenshot 2024-04-05 at 00 06 13

padding: 5px 8px;
font-size: 13px;
padding: 5px;
height: 20px;
Copy link
Owner

Choose a reason for hiding this comment

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

This fixed height gave me problems before, is this required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes applied in the base package through this merged PR https://github.com/maximebf/php-debugbar/pull/647/files should make sure it works well.

Screenshot 2024-04-05 at 14 05 05

The height of 16px is now being set to 20px as the minimum height of div.phpdebugbar-header in this package is 30px, compared to a min-height of 26px in the base package.

side-by-side

@barryvdh barryvdh merged commit 7a5e0c8 into barryvdh:master May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants