Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[WebProfilerBundle] Add HTTP return code in the Ajax request list table#17540

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
kucharovic wants to merge3 commits intosymfony:2.8fromkucharovic:fix_17518

Conversation

@kucharovic
Copy link
Contributor

QA
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#17518
LicenseMIT
Doc PR-

Q | A---|---Bug fix? | noNew feature? | yesBC breaks? | noDeprecations? | noTests pass? | yesFixed tickets |symfony#17518License | MITDoc PR | -
@javiereguiluz
Copy link
Member

@kucharovic I just tested your pull request and it works perfect! Thanks for working on this.

ajax_status_before

However, to make errors stand out more prominently, what do you think of using thesf-toolbar-status CSS classes that we use elsewhere in the toolbar. This way we could show the HTTP status like this:

ajax_status_after

If you like the idea, the code would be something like this:

varstatusCodeCell=document.createElement('td');varstatusCode=document.createElement('span');if(request.statusCode>299){statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-red');}else{statusCode.setAttribute('class','sf-toolbar-status');}statusCode.textContent=request.statusCode||'-';statusCodeCell.appendChild(statusCode);row.appendChild(statusCodeCell);

@kucharovic
Copy link
ContributorAuthor

@javiereguiluz it looks better. Are you sure, thatredirects should be red?

@javiereguiluz
Copy link
Member

@kucharovic I trustedthis comment from@Jean85 who said that in Ajax there are no redirections. We probably need more comments about this or some verification.

@stloyd
Copy link
Contributor

IMO it should be:

if(request.statusCode<300){statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-green');}elseif(request.statusCode<400){statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-yellow');}else{statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-red');}

@javiereguiluz
Copy link
Member

@kucharovic anyway, to be completely safe, we could changeif (request.statusCode > 299) byif (request.statusCode > 399) and this will always work no matter what.

@kucharovic
Copy link
ContributorAuthor

@javiereguiluzhe's right. A XMLHttpRequest should either get the resource or be told why not.

@stloyd
Copy link
Contributor

Hmmm, then maybe:

if(request.statusCode<400){statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-green');}elseif(request.statusCode<500){statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-yellow');}else{statusCode.setAttribute('class','sf-toolbar-status sf-toolbar-status-red');}

@javiereguiluz
Copy link
Member

@stloyd I like your proposal ... except for the last comment. Showing 4xx errors in yellow is inconsistent with the current toolbar. 4xx and 5xx are treated as errors and showed in red.

@Jean85
Copy link
Contributor

Yeah, I posted that comment because I stumbled on it.. I had an issue in my code, since I was trying to catch redirects withjQuery.ajax() and I was getting always 200, didn't understand why :D

I agree with@javiereguiluz , consistency with the rest of the toolbar is better.

@fabpot
Copy link
Member

👍

@stof
Copy link
Member

should we really put the 2xx in green or keep it in gray instead ? I think putting colors only on errors make them stand out more than if everything is colored

@javiereguiluz
Copy link
Member

For comparison:

Color on Errors

ajax_status_after

Color on Everything

ajax_status_after_colors

@fabpot
Copy link
Member

+1 for gray instead of green

@maidmaid
Copy link
Contributor

👍

@fabpot
Copy link
Member

Thank you@kucharovic.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@kucharovic@javiereguiluz@stloyd@Jean85@fabpot@stof@maidmaid@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp