alpine-collective / alpinejs-devtools

Chrome/Firefox DevTools extension for debugging Alpine.js applications.
MIT License
524 stars 18 forks source link

UI to surface Alpine.js eval errors #126

Closed HugoDF closed 3 years ago

HugoDF commented 3 years ago

Alpine.js uses console.warn in the following scenarios:

  1. x-if/x-for on a non-template https://github.com/alpinejs/alpine/blob/ee019cbc66d2adb3e5126bf3822d9d701a40daf8/src/utils.js#L29 #141
  2. template with more than 1 child node https://github.com/alpinejs/alpine/blob/ee019cbc66d2adb3e5126bf3822d9d701a40daf8/src/utils.js#L31 #141
  3. error during expression evaluation https://github.com/alpinejs/alpine/blob/5cc46c31b08e4b25821fb85fd0fad6903bc7e574/src/utils.js#L69

I believe some developers suppress console.warn, it would be nice to surface the relevant errors/warnings:

For 1. and 2. we should probably re-scan the DOM and to figure out which are the offending template's. Moved to a separate issue to have additional structural checks #141

For 3. the console.warn contains the element as well as the expression.

KevinBatdorf commented 3 years ago

Any idea where we should add various tabs? We could remove the "Alpine v2.7.1 detected" and add them there. Just a quick idea:

Screen Shot 2020-12-16 at 1 46 11 PM
<div class="inline-flex space-x-2 text-md">
  <div><button class="bg-alpine-300 hover:bg-alpine-300 leading-none px-3 py-2 rounded-full">Components</button></div>
  <div><button class="hover:bg-alpine-300 leading-none px-3 py-2 rounded-full">Events</button></div>
  <div><button class="hover:bg-alpine-300 leading-none px-3 py-2 rounded-full">Warnings</button></div>
</div>

We could also add a count for each button when not selected, like "Warnings 2" Or grab something from tailwindui?

I also noticed a white gap bottom right of the right pane. I'll see what that is later

HugoDF commented 3 years ago

We could do tabs like you've done but to the right, before the GitHub/docs/settings icons?

KevinBatdorf commented 3 years ago

Here's how that looks:

Screen Shot 2020-12-16 at 4 24 24 PM

It could be moved over some more too. The "docs" icon has some extra padding too that should probably be removed to match the other icons.

Is it just me or does the "Alpine.js v2.7.3 detected" feel too overwhelming? It's not really that critical of information besides the version number, which is really only critical in a situation where you're trying to debug in support

KevinBatdorf commented 3 years ago

Also, i'm not a great designer, so if someone has an opinion on the button styles, etc. please do share it. :)

ryangjchandler commented 3 years ago

I went with the right aligned tabs (not pill shapes though) in #129 (still a WIP). Probably makes more sense, I tend to naturally float to the right when searching for buttons.

HugoDF commented 3 years ago

Yeah maybe we can shorten the text from "Alpine.js v2.7.3 detected" to circle icon + "v2.7.3" and maybe use a lighter shade of orange?

We can also compress the pills/tabs for small breakpoints by switching to an icon + title attribute

KevinBatdorf commented 3 years ago

Ah, I didn't check #​129 yet. All that sounds good. Right side sounds good too. Choosing the right icons might be the challenge now :)

I think we also need room for a counter too. For example if there are 3 warnings or 5 events. Or if not a counter, just a colored dot might be enough to show there's some activity there.

Screen Shot 2020-12-16 at 5 15 26 PM
KevinBatdorf commented 3 years ago

We also need a loading state for the buttons. Or otherwise just make the toolbar always dark?

Screen Shot 2020-12-16 at 5 17 00 PM
HugoDF commented 3 years ago

We also need a loading state for the buttons. Or otherwise just make the toolbar always dark?

Screen Shot 2020-12-16 at 5 17 00 PM

yeah I think this looks fine, we can disable/fade or even hide them while loading

stephenoldham commented 3 years ago

Not to put the cat amongst the pigeons πŸ˜‰

Here's my take:

AlpineDevTools_Concept

* Don't get me started on those sweeeet looking themes πŸ‘ŒπŸΌπŸ˜

stephenoldham commented 3 years ago

Or does the right align feel right?! πŸ˜¬πŸ˜† Back to @KevinBatdorf's original shout.

AlpineDevTools_Concept_v2

KevinBatdorf commented 3 years ago

I like it! And rip off anything :)

Maybe we can add a theming setup to this project eventually as well.

Looking at both, right-aligned seems nicer. Maybe we could move those three icons to the right footer as well.

ryangjchandler commented 3 years ago

I personally prefer the right aligned items, feels less crowded on the left hand side where the list of components, etc will be.

@stephenoldham is the design master mind though ;)

stephenoldham commented 3 years ago

Ha! Not sure about that @ryangjchandler πŸ˜†

Big yes from me on the theming setup @KevinBatdorf πŸ‘πŸΌ Would be a rad addition to the settings layer.

The only concern might be the settings getting lost on the footer but see what you think:

AlpineDevTools_Concept_v1 1

Here's a few footer vibes...you get the idea:

Screenshot 2020-12-17 at 15 57 58 Screenshot 2020-12-17 at 15 57 41 Screenshot 2020-12-17 at 15 57 34 Screenshot 2020-12-17 at 15 56 59
ryangjchandler commented 3 years ago

The 2nd one in is my favourite. The separator is nice, but I also like the last one with the different BG colour.

KevinBatdorf commented 3 years ago

I like them all. Divider with different BG color looks nice.

Maybe put the settings icon all the way to the right so it stands out slightly more (does it, I'm not necessarily sure)?

We can always move it later too

ryangjchandler commented 3 years ago

If somebody is going to implement the tabs, etc, I'll hold off on #129.

HugoDF commented 3 years ago

Ha! Not sure about that @ryangjchandler πŸ˜†

Big yes from me on the theming setup @KevinBatdorf πŸ‘πŸΌ Would be a rad addition to the settings layer.

The only concern might be the settings getting lost on the footer but see what you think:

AlpineDevTools_Concept_v1 1

Here's a few footer vibes...you get the idea:

Screenshot 2020-12-17 at 15 57 58 Screenshot 2020-12-17 at 15 57 41 Screenshot 2020-12-17 at 15 57 34 Screenshot 2020-12-17 at 15 56 59

😍 could try with the status bar (Alpine.js vX.X.X detected) between the header and the main panes?

I was happy to settle for text links in the header πŸ˜„

stephenoldham commented 3 years ago

Right then...the contenders...

AlpineDevTools_Concept_v1 2 AlpineDevTools_Concept_v2

Quite like both πŸ˜† Subheader version would just need a bit of play with the padding on the right. Footer version I think I prefer the outline icons without the background. Thoughts?

HugoDF commented 3 years ago

Subheader version will give use the opportunity to bring in a search UI maybe?

I guess they're very similar either way (plus I'm sure we can move it around if we decide on one and end up not liking it)

ryangjchandler commented 3 years ago

I'm gonna vote for the footer version.

stephenoldham commented 3 years ago

I think in terms of the content hierarchy I prefer the footer. Also feels like there's more space in that layout with less at the top.

@HugoDF As you said we can totally change it if we don't vibe that layout in practise.

HugoDF commented 3 years ago

Yeah it's fine, I'm a bit weary of making big changes to the layout of the UI if people have come to rely on it.

Then again, better to make the change now to support future features rather than wait

ryangjchandler commented 3 years ago

Anybody volunteering to implement the new design?

stephenoldham commented 3 years ago

@ryangjchandler Can do πŸ‘πŸΌ

Unless anyone else is up for it.

HugoDF commented 3 years ago

cool, I would say just to watch out for the testid's otherwise tests will fail and also to hide tabs that don't have any content for production builds (the same way we hid the settings modal #111)

HugoDF commented 3 years ago

Additional things we can write our own "warning" for (see https://github.com/alpinejs/alpine/discussions/975)

Thinking about it further the following:

Don't need to/can't be done with console.warn interception, so it might be worth splitting as a separate issue.

Other note: I'm not sure how we're going to highlight template's that have issues (since they don't visually display)

HugoDF commented 3 years ago

We might not need to instrument console.warn once https://github.com/alpinejs/alpine/pull/1000 gets released

Edit: got released under 2.8.1, can do a PR for support, will also add a "no eval warnings under 2.8.0" state (for now until we add the markup lint stuff)