Skip to content

Redesigned the log message filter #28906

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 19, 2018
Merged

Redesigned the log message filter #28906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@
var bullet = document.createElement('li');
bullet.innerText = level;
bullet.setAttribute('data-log-level', String(i));
bullet.setAttribute('title', 'Show logs of level ' + level + ' or higher');
bullets.appendChild(bullet);
addEventListener(bullet, 'click', function() {
if (i === this.parentNode.querySelectorAll('.active').length - 1) {
Expand Down
45 changes: 40 additions & 5 deletions src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,9 @@ tr.status-warning td {
background-color: var(--base-5);
color: var(--base-2);
}
.tab-content {
position: relative;
}
.tab-content > *:first-child {
margin-top: 0;
}
Expand Down Expand Up @@ -957,11 +960,43 @@ table.logs .metadata {
}
table.logs tr td:last-child { width: 100%; }

.log-levels { width: 100%; margin: 0; padding: 0; display: flex; align-items: center; list-style: none; }
.log-levels li { width: 100%; padding: 3px; margin: 0; cursor: pointer; text-align: center; border: 2px dashed #e0e0e0; border-radius: 5px; color: #888; }
.log-levels li + li { margin-left: 10px; }
.log-levels li.active { background: #eee; color: #666; border-style: solid; border-width: 1px; padding: 4px; border-color: #aaa; }
.log-levels li.last-active { cursor: not-allowed; }
.log-levels { position: absolute; right: 5px; top: 33px; }
.log-levels { border: var(--border); box-shadow: var(--shadow); margin: 0; padding: 0; display: flex; flex-direction: column; align-items: flex-end; }
.log-levels:before {
content: "Filter";
Copy link
Contributor

@ro0NL ro0NL Oct 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

perhaps Level is better (given we already know it's filter by icon), that would enable a second Channel filter as well (which was on my list). Unless you want to combine things here of course :)

perhps log-filters for a class, and move the content inline

Copy link
Member Author

Choose a reason for hiding this comment

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

@ro0NL I propose you something. Given that we can no longer add a filter for the log channel in 4.2 (the "feature freeze" was a long ago, we cannot keep making changes) let's keep this filter "as is".

Later, for 4.3, we can add filters in several columns (and not only for logs). I was thinking something like GitHub filters:

github-filters

Or something like "advanced data grids" which allow multi-filtering per column:

multiple-filters

Copy link
Member

Choose a reason for hiding this comment

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

We can still change things in 4.2, the 2 month period is to tweak things that are already part of the release, which is the case here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes ... but in my opinion what Roland is proposing (table multi-filtering) would require lots of changes. @ro0NL if you think you can do it in time for 4.2, please close this PR and implement the multi-filtering inside the table columns. Thanks 🙏 !

Copy link
Contributor

Choose a reason for hiding this comment

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

i thought it could share the same JS basically, but that's not the case (yet).

Channel filters doesnt need the level approach (selecting all higher levels). So yeah, maybe that's something for 4.3, we'll see :)

Still, using Levels for a label makes sense to let the user know what this filter is about ;)

Copy link
Member

Choose a reason for hiding this comment

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

If that's too much work, it then makes sense to improve what we have for 4.2 and do more/better in 4.3.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've discussed about this with @ro0NL on Symfony Slack. We agree to leave this "as is" for Symfony 4.2 (so this PR is OK to be merged).

In Symfony 4.3 we strive to create a multi-purpose filter logic to add filters to all/most column headers of all/most tables. Thanks!

cursor: pointer;
/* "filter" icon provided by FontAwesome - CC BY 4.0 License
https://github.com/FortAwesome/Font-Awesome/blob/master/LICENSE.txt */
background: var(--table-background) no-repeat url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path fill="%23999" d="M487.976 0H24.028C2.71 0-8.047 25.866 7.058 40.971L192 225.941V432c0 7.831 3.821 15.17 10.237 19.662l80 55.98C298.02 518.69 320 507.493 320 487.98V225.941l184.947-184.97C520.021 25.896 509.338 0 487.976 0z" class=""></path></svg>');
background-position: 6px 6px;
background-size: 14px 14px;
padding: 4px 10px 4px 24px;
position: absolute;
top: -28px;
}
.log-levels:hover li { display: inline-flex; }
.log-levels li {
background: var(--tab-disabled-background);
border-bottom: var(--border);
color: var(--tab-disabled-color);
cursor: s-resize;
display: none;
list-style: none;
margin: 0;
padding: 5px 10px;
text-align: left;
width: 100%;
}
.log-levels li.active {
background: var(--tab-background);
color: var(--tab-color);
cursor: n-resize;
}
.log-levels li.last-active {
background: var(--tab-active-background);
color: var(--tab-active-color);
cursor: unset;
}

{# Doctrine panel
========================================================================= #}
Expand Down