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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quality of life improvements in Requests tab #537

Merged
merged 5 commits into from Mar 16, 2022
Merged

Quality of life improvements in Requests tab #537

merged 5 commits into from Mar 16, 2022

Conversation

glujan
Copy link
Contributor

@glujan glujan commented Jan 5, 2022

I'm using you're package quite often and wanted to give something back to this project 馃槃

This PR focuses on improving experience on Request tab - one improvement per commit:

  • Add message when there are no requests to display: when there are no matches instead of displaying an empty page, let user know that there are no filter results
  • Add 'Clear all filters' button - when user fills in a few filters, clearing them is rather tedious task so I thought it would be nice to have a button to clear filters.
  • Add small margin for filter selects: I think this change makes filter section looks much better.

This branch is based off 4.2.0 tag, please let me know if I need to rebase.

@auvipy
Copy link
Contributor

auvipy commented Jan 6, 2022

thank your for your contributions, lets see what the CI says :D

@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #537 (6d8204e) into master (8f8d159) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #537   +/-   ##
=======================================
  Coverage   84.59%   84.59%           
=======================================
  Files          51       51           
  Lines        2058     2058           
=======================================
  Hits         1741     1741           
  Misses        317      317           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 8f8d159...6d8204e. Read the comment docs.

@auvipy
Copy link
Contributor

auvipy commented Jan 6, 2022

can you please share the screenshots of UI that changed with this PR?

@glujan
Copy link
Contributor Author

glujan commented Jan 6, 2022

Updated filters:

Filters

New message saying there are no matching results:

No matches found

Changes not included in this PR

I also have some other changes to CSS which makes text inputs in filters easier to see (some of my colleagues did not notice that you can use filters like Executed n seconds ago). I can include those changes in the PR if you like it but of course for me it's fine to keep it as overrides in my project(s):
obraz

@auvipy
Copy link
Contributor

auvipy commented Jan 6, 2022

you can add the other improvements to this PR

@glujan
Copy link
Contributor Author

glujan commented Jan 10, 2022

@auvipy I've added those additional changes, the PR is good to be reviewed now.

Copy link
Member

@albertyw albertyw left a comment

Choose a reason for hiding this comment

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

Looks good to me too. Thanks!

@auvipy
Copy link
Contributor

auvipy commented Jan 11, 2022

This branch is based off 4.2.0 tag, please let me know if I need to rebase.

that should target main branch

@glujan
Copy link
Contributor Author

glujan commented Jan 11, 2022

PR was already targeting master branch, but now my branch is rebased against master branch.

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

btw, should'nt we mention the UI changes in the docs where necessary?

@glujan
Copy link
Contributor Author

glujan commented Jan 11, 2022

btw, should'nt we mention the UI changes in the docs where necessary?

@auvipy I can add some docs regarding what has changed but I cannot see any section about filtering in the docs. Could you point me where I should add it?

There is also a changelog file, should I update it or will it be updated when creating a new release?

@glujan
Copy link
Contributor Author

glujan commented Feb 10, 2022

Hi @auvipy , should I do anything more to get this merged? I'm happy to improve this PR to meet your - and the whole Jazz Band's - expectations!

@albertyw
Copy link
Member

There's some docs under https://github.com/jazzband/django-silk/tree/master/docs/ which could be updated but none of them are invalidated by this change so I'll merge this as is.

@albertyw albertyw merged commit 6df8ac0 into jazzband:master Mar 16, 2022
@glujan glujan deleted the Requests-tab-improvements branch March 16, 2022 07:36
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.

None yet

4 participants