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

Mark exceptions as resolved #710

Merged
merged 3 commits into from
Aug 30, 2019

Conversation

barryvdh
Copy link
Contributor

This allows to mark exceptions as resolved, and show warnings when not resolved.

image

image

If an Exception occurs again, it's shown as not-resolved again (because the last entry is shown, which is good).

@taylorotwell
Copy link
Member

The button looks slightly too low. It's not vertically aligned with the text on the left.

@barryvdh
Copy link
Contributor Author

Is this better?

image

@taylorotwell
Copy link
Member

Hmm, it looks more aligned but now the top padding on the button appears much less than the bottom padding. Sorry I am picky about these things 😬

@barryvdh
Copy link
Contributor Author

But isn't that exactly the same as the Auth tag? Tried using the built in styling classes.

@barryvdh
Copy link
Contributor Author

Do you mean inside the button? Because the padding above the lowercase letters it the same top and bottom. Only the uppercase 'R' and 'l' are closer to the top. So not really sure how it should be aligned otherwise:
image

@GertjanRoke
Copy link

GertjanRoke commented Aug 29, 2019

Maybe the vertical-align of the .badge class needs to be changed to vertical-align: middle; that fixed it for me.

@taylorotwell
Copy link
Member

@GertjanRoke can you screenshot that?

@GertjanRoke
Copy link

GertjanRoke commented Aug 29, 2019

image
The one on the right is with vertical-align: middle;

I saw that Github does the same with there badges.

@barryvdh
Copy link
Contributor Author

I can change that, but that would be benifitial for all .badge items, so not sure if that's related to this PR.

@GertjanRoke
Copy link

@taylorotwell I thought I would make a better screenshot for you by rebuilding what @barryvdh had in his first screenshot.

So this one is with vertical-align: bottom:

image

And this one is with vertical-align: middle:

image

And this is with the current styling:

image

Hope this makes it easier to decide what the best option is.

@taylorotwell taylorotwell merged commit 52d5456 into laravel:2.0 Aug 30, 2019
@taylorotwell
Copy link
Member

I'll just go as-is for now.

@barryvdh barryvdh deleted the feat-resolve-exceptions branch August 30, 2019 09:01
@barryvdh
Copy link
Contributor Author

Thanks, should I submit a PR to rebuild assets and perhaps tag a new release after that, to prepare for 6.0?

macleash90 added a commit to macleash90/telescope that referenced this pull request Jul 21, 2020
… are marked as resolved the message isn't clear to the user. For once I thought Laravel Telescope would itself automagically resolve the exception for me since that's what the message suggested (Resolve now). This change is to indicate to users that exceptions aren't resolved by Laravel Telescope but marked as resolved, so they have to go resolve the exception themselves and mark as resolved on Laravel Telescope which is the intention of pull request laravel#710
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.

3 participants