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

Inspector UI Fixes #6119

Merged
merged 18 commits into from
Jul 29, 2024
Merged

Inspector UI Fixes #6119

merged 18 commits into from
Jul 29, 2024

Conversation

lankaukk
Copy link
Collaborator

@lankaukk lankaukk commented Jul 24, 2024

Making a number of updates to the inspector UI:

  • reordering the "container" section to be below the frame section
  • when the border-radius and padding controls are split into 4 inputs, the last 2 move onto another row
  • the split input control always stays in the right place
  • when a dropdown is not hovered, the v expansion arrow stays at the end of the text, and moves to the far right of the input only when it is hovered
  • the opacity control is not a scrubbable input, no longer a slider
  • blend mode is always visible
  • the overflow toggle is removed (since we already have clip content)
  • the alignment buttons are above the style section
  • other various padding and border styling tweaks

BK:

  • updated the tests to use the opacity input instead of the opacity slider
  • updated the tests to look for the Background label instead of Container (which was removed)
Before After
Screenshot 2024-07-25 at 5 38 28 PM Screenshot 2024-07-25 at 5 38 49 PM
Screenshot 2024-07-25 at 5 41 07 PM Screenshot 2024-07-25 at 5 40 12 PM
Screenshot 2024-07-25 at 5 39 44 PM Screenshot 2024-07-25 at 5 40 39 PM
Screenshot 2024-07-25 at 5 42 43 PM Screenshot 2024-07-25 at 5 42 09 PM
25-43-gxom7-gg29c 25-43-crhaz-khuwd

Manual Tests:
I hereby swear that:

  • I opened a hydrogen project and it loaded
  • I could navigate to various routes in Preview mode

Copy link
Contributor

github-actions bot commented Jul 24, 2024

Try me

Copy link

relativeci bot commented Jul 24, 2024

#13576 Bundle Size — 62.65MiB (~-0.01%).

c5258e2(current) vs 6339ece master#13575(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics  Change 3 changes Improvement 1 improvement
                 Current
#13576
     Baseline
#13575
Improvement  Initial JS 45.73MiB(-0.01%) 45.73MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 28.12% 21.57%
No change  Chunks 31 31
No change  Assets 34 34
Change  Modules 4377(-0.07%) 4380
No change  Duplicate Modules 525 525
No change  Duplicate Code 31.64% 31.64%
No change  Packages 469 469
No change  Duplicate Packages 70 70
Bundle size by type  Change 2 changes Improvement 2 improvements
                 Current
#13576
     Baseline
#13575
Improvement  JS 62.64MiB (~-0.01%) 62.65MiB
Improvement  HTML 11.16KiB (-0.33%) 11.2KiB

Bundle analysis reportBranch inspector-uiProject dashboard

@lankaukk lankaukk marked this pull request as ready for review July 25, 2024 21:50
@maltenuhn maltenuhn self-requested a review July 29, 2024 11:54
Copy link
Member

@maltenuhn maltenuhn left a comment

Choose a reason for hiding this comment

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

Can we change the four-paddings control order from 'TR/BL' to 'LT/RB' per Discord?

minimum={0}
maximum={1}
stepSize={0.01}
inputProps={{ onMouseDown: (e) => e.stopPropagation() }}
Copy link
Contributor

Choose a reason for hiding this comment

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

minor - but this could be a useCallback just to prevent re-rendering

return (
<FlexRow style={style}>
<FlexRow style={{ ...style, flexWrap: wrap ? 'wrap' : 'nowrap' }}>
Copy link
Contributor

@liady liady Jul 29, 2024

Choose a reason for hiding this comment

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

very minor - but better to do {{ flexWrap: wrap ? 'wrap' : 'nowrap', ...style }} so that the consumer will always be able to override (for example if they pass style={{ flexWrap: 'wrap'}} but no wrap prop)

@lankaukk lankaukk merged commit ca05c44 into master Jul 29, 2024
13 checks passed
@lankaukk lankaukk deleted the inspector-ui branch July 29, 2024 16:09
liady pushed a commit that referenced this pull request Dec 13, 2024
Making a number of updates to the inspector UI:
- reordering the "container" section to be below the frame section
- when the border-radius and padding controls are split into 4 inputs,
the last 2 move onto another row
- the split input control always stays in the right place
- when a dropdown is not hovered, the `v` expansion arrow stays at the
end of the text, and moves to the far right of the input only when it is
hovered
- the opacity control is not a scrubbable input, no longer a slider
- blend mode is always visible
- the overflow toggle is removed (since we already have clip content)
- the alignment buttons are above the style section
- other various padding and border styling tweaks

BK:
- updated the tests to use the opacity input instead of the opacity
slider
- updated the tests to look for the `Background` label instead of
`Container` (which was removed)

| Before  | After |
| ------------- | ------------- |
| <img width="289" alt="Screenshot 2024-07-25 at 5 38 28 PM"
src="https://github.com/user-attachments/assets/9fe8fa32-2f56-415e-91e2-f606a98bb7ba">
| <img width="303" alt="Screenshot 2024-07-25 at 5 38 49 PM"
src="https://github.com/user-attachments/assets/8fdedddf-08e1-4806-b4d9-947089ecdd45">
|
| <img width="274" alt="Screenshot 2024-07-25 at 5 41 07 PM"
src="https://github.com/user-attachments/assets/c6d7a264-c9a5-4eb8-9e76-9c69684ae51d">
| <img width="274" alt="Screenshot 2024-07-25 at 5 40 12 PM"
src="https://github.com/user-attachments/assets/6fa10dd2-bbd3-4243-9faf-0fe4488a9836">
|
| <img width="278" alt="Screenshot 2024-07-25 at 5 39 44 PM"
src="https://github.com/user-attachments/assets/262803d4-c308-4689-8e4a-19e340c8d231">
| <img width="269" alt="Screenshot 2024-07-25 at 5 40 39 PM"
src="https://github.com/user-attachments/assets/2c6b5796-337e-4cbc-a922-9974b34f66fa">
|
| <img width="274" alt="Screenshot 2024-07-25 at 5 42 43 PM"
src="https://github.com/user-attachments/assets/c79130dd-722c-4630-835c-d2a5e354b203">
| <img width="269" alt="Screenshot 2024-07-25 at 5 42 09 PM"
src="https://github.com/user-attachments/assets/ff6446f2-fd9a-4208-815e-853521239cfb">
|
|
![25-43-gxom7-gg29c](https://github.com/user-attachments/assets/2be5e421-096a-4e93-baa8-119898c299d5)
|
![25-43-crhaz-khuwd](https://github.com/user-attachments/assets/03481ca0-400d-4dd8-87bc-0da372186f4f)
|

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Preview mode

---------

Co-authored-by: Berci Kormendy <[email protected]>
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.

7 participants