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

[WebProfilerBundle] Log section minor fixes (missing "notice" filter, log priority, accessibility) #45351

Closed
wants to merge 0 commits into from

Conversation

Amunak
Copy link
Contributor

@Amunak Amunak commented Feb 8, 2022

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #42195 (related)
License MIT
Doc PR ~

This fixes minor front-end issues with the redesigned Log section in WebProfilerBundle.

Marking as bugfix, since technically the redesign removed some features without documenting it - the log level is now visible even for low level logs (debug, notice, info). I have also added the missing "notice" filter and made sure that when a filter is missing the message is shown (fail-safe).

I have also implemented accessibility fixes mentioned in the original PR (#42195) by @stof.

I did not add tests because I have no idea where or how - currently the profiler front-end is not tested (or at least the log section). These changes do not affect the logic behind the panel in a way that could be easily tested.

Screenshot of the changes: notice the visible log levels and added "notice" filter:

Screenshot of the changes

@nicolas-grekas nicolas-grekas added this to the 5.4 milestone Feb 9, 2022
@carsonbot carsonbot changed the title [WebProfilerBundle] Log section minor fixes (missing "notice" filter, log priority, accessibility) Log section minor fixes (missing "notice" filter, log priority, accessibility) Feb 9, 2022
@carsonbot carsonbot changed the title Log section minor fixes (missing "notice" filter, log priority, accessibility) [WebProfilerBundle] Log section minor fixes (missing "notice" filter, log priority, accessibility) Feb 9, 2022
@fabpot
Copy link
Member

@fabpot fabpot commented Feb 25, 2022

Copy link
Member

@javiereguiluz javiereguiluz left a comment

These fixes are very nice! Thanks Jiří

@fabpot
Copy link
Member

@fabpot fabpot commented Feb 25, 2022

Thank you @Amunak.

fabpot added a commit that referenced this issue Feb 25, 2022
…ce" filter, log priority, accessibility) (Amunak)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[WebProfilerBundle] Log section minor fixes (missing "notice" filter, log priority, accessibility)

| Q             | A
| ------------- | ---
| Branch?       | 5.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #42195 (related)
| License       | MIT
| Doc PR        | ~

This fixes minor front-end issues with the redesigned Log section in WebProfilerBundle.

Marking as bugfix, since technically the redesign removed some features without documenting it - the log level is now visible even for low level logs (debug, notice, info). I have also added the missing "notice" filter and made sure that when a filter is missing the message is shown (fail-safe).

I have also implemented accessibility fixes mentioned in the original PR (#42195) by @stof.

I did not add tests because I have no idea where or how - currently the profiler front-end is not tested (or at least the log section). These changes do not affect the logic behind the panel in a way that could be easily tested.

Screenshot of the changes: notice the visible log levels and added "notice" filter:

![Screenshot of the changes](https://user-images.githubusercontent.com/781546/152992378-89452512-7b94-40b7-9d8e-8f94acc4d058.png)

Commits
-------

1c3b266 [WebProfilerBundle] Log section minor fixes (missing "notice" filter, log priority, accessibility)
@fabpot fabpot closed this Feb 25, 2022
@fabpot fabpot force-pushed the 42195-profiler-fixes branch from 95dda72 to 1c3b266 Feb 25, 2022
@fabpot fabpot mentioned this pull request Feb 28, 2022
@fabpot fabpot mentioned this pull request Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants