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

Fix Content-Types and Content-Parts search UI #16237

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

MikeAlhayek
Copy link
Member

@MikeAlhayek MikeAlhayek commented Jun 4, 2024

Before this, if you click enter in the text box you'll get a 404. Also, the keyCode property on the JavaScript is deprecated.

Also, now we no longer need jQuery for this to work. And the logic is reusable.

Here is a quick demo of the UI behavior for both content-types and content-parts UI.

content-types-list

@MikeAlhayek MikeAlhayek requested a review from Piedone June 4, 2024 22:41
@Piedone
Copy link
Member

Piedone commented Jun 4, 2024

I see the Enter issue, but not the Esc one. On main, if I hit Esc when searching, then the search box will be cleared and the filtering removed:

2024-06-05_01h35_45.mp4

This is exactly what I'd expect to happen. What behavior is this fixing for Esc?

@MikeAlhayek
Copy link
Member Author

MikeAlhayek commented Jun 5, 2024

I don't think there was an issue with escape actually. There is a deprecated warning. But the enter is definitely a problem. I updated the description.

Let me know if you see an issue with the current PR.

@Piedone
Copy link
Member

Piedone commented Jun 5, 2024

I'll review it soon.

@Piedone
Copy link
Member

Piedone commented Jun 5, 2024

I still get a 404 on Enter:

2024-06-05_23h20_16.mp4

@MikeAlhayek
Copy link
Member Author

I still get a 404 on Enter:

I am guessing you need to do a deep refresh CTRL + F5. It works with no issues for me (just tried it out again). Look at the video in the description to see how it works for me.

@Piedone
Copy link
Member

Piedone commented Jun 5, 2024

I did that. With the DevTools open the cache is disabled anyway.

@MikeAlhayek
Copy link
Member Author

Can you please check the generated HTML? ensure that the old script is removed and the list-items-filter.js is included

@Piedone
Copy link
Member

Piedone commented Jun 5, 2024

Now it works, WTF. Anyway, I'll continue with the code review.

@MikeAlhayek MikeAlhayek merged commit 6911193 into main Jun 5, 2024
11 checks passed
@MikeAlhayek MikeAlhayek deleted the ma/content-types-search-ui branch June 5, 2024 23:43
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