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

admin: Add support for comment search by Thread URL in admin interface #1020

Merged

Conversation

pkvach
Copy link
Contributor

@pkvach pkvach commented Apr 27, 2024

Checklist

  • All new and existing tests are passing
  • I have added an entry to CHANGES.rst because this is a user-facing change or an important bugfix
  • I have written proper commit message(s)

What changes does this Pull Request introduce?

The main theme of the changes is to enhance the comment search functionality in the admin interface. The changes allow searching for comments not only by comment URL but also by thread URL.

Why is this necessary?

@pkvach pkvach force-pushed the feat/admin-comment-search-by-thread-url branch from 6fed88b to ba8dffb Compare April 27, 2024 22:07
Copy link
Member

@ix5 ix5 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Could we perhaps find a better way to convey to admins that they can enter both thread and individual comment URLs?

Also, would it make sense to make the search field a bit smarter and just let it accept plain numbers (for comment ids) as well?

isso/templates/admin.html Outdated Show resolved Hide resolved
isso/templates/admin.html Outdated Show resolved Hide resolved
isso/templates/admin.html Outdated Show resolved Hide resolved
isso/views/comments.py Outdated Show resolved Hide resolved
@pkvach
Copy link
Contributor Author

pkvach commented Apr 29, 2024

Thanks for the PR!

I'm happy to help :)

Could we perhaps find a better way to convey to admins that they can enter both thread and individual comment URLs?

There is an option to add a tooltip with explanatory text. This will not take up too much space.

Also, would it make sense to make the search field a bit smarter and just let it accept plain numbers (for comment ids) as well?

Then we will definitely need explanatory text and will have to give up <input type=“url”>.

@ix5
Copy link
Member

ix5 commented Apr 29, 2024

Could we perhaps find a better way to convey to admins that they can enter both thread and individual comment URLs?

There is an option to add a tooltip with explanatory text. This will not take up too much space.

Good idea. Something like <abbr> that pops on mouseover will not work on mobile, but if you have a good solution, go for it.

Also, would it make sense to make the search field a bit smarter and just let it accept plain numbers (for comment ids) as well?

Then we will definitely need explanatory text and will have to give up <input type=“url”>.

On second thought, let's not implement the "smart" search and leave it as is. It would just be confusing.

@pkvach
Copy link
Contributor Author

pkvach commented May 2, 2024

I added a tooltip with help text next to the search field.
It's ready for review.

Copy link
Member

@ix5 ix5 left a comment

Choose a reason for hiding this comment

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

Nice, the tooltip is really useful.

I'd just want it a bit more integrated/look alike the rest of the surrounding buttons, so that it's clearer it is clickable.

Current:
Screenshot from 2024-05-03 19-03-18

With more prominent border and shadow:
Screenshot from 2024-05-03 19-02-55

isso/css/admin.css Show resolved Hide resolved
isso/css/admin.css Show resolved Hide resolved
isso/css/admin.css Outdated Show resolved Hide resolved
@pkvach pkvach force-pushed the feat/admin-comment-search-by-thread-url branch from 9c53080 to bf346d1 Compare May 4, 2024 07:07
@pkvach
Copy link
Contributor Author

pkvach commented May 4, 2024

Nice, the tooltip is really useful.

I'd just want it a bit more integrated/look alike the rest of the surrounding buttons, so that it's clearer it is clickable.

Current: Screenshot from 2024-05-03 19-03-18

With more prominent border and shadow: Screenshot from 2024-05-03 19-02-55

Thank you for the review and a nice addition.
I made the suggested changes and did squash commits.

@ix5 ix5 merged commit 0b7e6fe into isso-comments:master May 4, 2024
18 checks passed
@ix5
Copy link
Member

ix5 commented May 4, 2024

Merged, thanks!

@pkvach pkvach deleted the feat/admin-comment-search-by-thread-url branch May 4, 2024 19:06
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.

2 participants