News Navigator Plugin #2557
News Navigator Plugin #2557
Conversation
Signed-off-by: Lucifergene <[email protected]>
Lucifergene
requested review from
backstage-service and
a team
as code owners
Lucifergene
changed the title
Changed Packages
|
There was a problem hiding this comment.
Hi @Lucifergene, thanks for that plugin!
To be honest, I'm not sure if this is a plugin for this community. That's a topic on it's own...
Or if it's possible to support multiple news providers.
But I like to share some feedback on your code anyway.
To be transparent: I worked with @Lucifergene before and shared most if this feedback in a short call already.
| csp: | ||
| connect-src: ["'self'", 'http:', 'https:'] |
There was a problem hiding this comment.
That's one of the reasons I'm unsure if backstage is a good candidate to have this as a plugin. Maybe it's an option to build this as an entire app (think of it as a container image) where users just need one enviroment variable to have a news app running?
You can also drop all other parts like catalog, search, kubernetes from that app. Just an idea.
| "scripts": { | ||
| "dev": "yarn workspaces foreach -A --include @backstage-community/plugin-news-navigator-backend --include @backstage-community/plugin-news-navigator --parallel -v -i run start", | ||
| "tsc": "tsc", | ||
| "tsc:full": "tsc --skipLibCheck false --incremental false", |
There was a problem hiding this comment.
Your build fails with an backstage/cli TS issue. You might want ignore node_modules from type checks:
| "tsc:full": "tsc --skipLibCheck false --incremental false", | |
| "tsc:full": "tsc --skipLibCheck true --incremental false", |
| * Configuration for the NewsAPI plugin. | ||
| * @visibility secret | ||
| */ | ||
| newsAPI?: { |
There was a problem hiding this comment.
I'm just curious that this is newsAPI and not News navigator. I would expect that both matches, also if this isn't required.
| "express": "^4.17.1", | ||
| "express-promise-router": "^4.1.0", | ||
| "newsapi": "^2.4.1", | ||
| "zod": "^3.22.4" |
There was a problem hiding this comment.
I think you doesn't use zod, or?
| return newsapi.v2 | ||
| .topHeadlines({ | ||
| category, | ||
| country: 'us', |
There was a problem hiding this comment.
Maybe you want make that configurable in the app-config.yaml as well?
| {[ | ||
| { path: '/', title: 'General' }, | ||
| { path: '/science', title: 'Science' }, | ||
| { path: '/technology', title: 'Technology' }, | ||
| { path: '/health', title: 'Health' }, | ||
| { path: '/sports', title: 'Sports' }, | ||
| { path: '/business', title: 'Business' }, | ||
| { path: '/entertainment', title: 'Entertainment' }, |
There was a problem hiding this comment.
Maybe you want allow users to specify which categories are shown? With this as default of course.
| const { data, isError, isLoading, isRefetching } = useQuery({ | ||
| queryKey: category ? ['category-news'] : ['search-news'], | ||
| queryFn: () => | ||
| category | ||
| ? newsApi.getNewsByCategory(category) | ||
| : newsApi.searchNewsByKeyword(keyword), | ||
| staleTime: 30 * 60 * 1000, // Cache data for 30 minutes | ||
| }); |
There was a problem hiding this comment.
Here is your issue that you're code needs to invalidate data. You should use the category and keyword value as part of the queryKey, like:
| const { data, isError, isLoading, isRefetching } = useQuery({ | |
| queryKey: category ? ['category-news'] : ['search-news'], | |
| queryFn: () => | |
| category | |
| ? newsApi.getNewsByCategory(category) | |
| : newsApi.searchNewsByKeyword(keyword), | |
| staleTime: 30 * 60 * 1000, // Cache data for 30 minutes | |
| }); | |
| const { data, isError, isLoading, isRefetching } = useQuery({ | |
| queryKey: category ? ['category-news', category] : ['search-news', keyword], | |
| queryFn: () => | |
| category | |
| ? newsApi.getNewsByCategory(category) | |
| : newsApi.searchNewsByKeyword(keyword), | |
| staleTime: 30 * 60 * 1000, // Cache data for 30 minutes | |
| }); |
| staleTime: 30 * 60 * 1000, // Cache data for 30 minutes | ||
| }); | ||
|
|
||
| if (isLoading || isRefetching) { |
There was a problem hiding this comment.
I would not jump back to the progress indicator when refetching happen, but that might be just an optinion:
| if (isLoading || isRefetching) { | |
| if (isLoading) { |
| const handleKeyPress = (event: { key: string }) => { | ||
| if (event.key === 'Enter') { | ||
| setSearchSubmitted(true); | ||
| queryClient.invalidateQueries({ queryKey: ['search-news'] }); |
There was a problem hiding this comment.
Should not be needed, see comment above.
| queryClient.invalidateQueries({ queryKey: ['search-news'] }); |
| const [searchText, setSearchText] = useState(''); | ||
| const [searchSubmitted, setSearchSubmitted] = useState(false); |
There was a problem hiding this comment.
Instead of useState I like to recommend useQueryParamState from Backstage core-components.
Signed-off-by: Avik Kundu [email protected]
New year, new plugin 🎉
Introducing the News Navigator plugin!
This simple plugin helps you stay informed with the latest news updates across categories like sports, politics, technology, and more. Why would you want a news plugin, you ask? Staying up to date is crucial in today’s fast-paced world, and this plugin makes it easy to explore top stories or dive deep into specific topics with keyword-based searches. Whether you’re catching up on headlines or exploring trends, this tool keeps you in the know.
Tab-22-01-2025_00_12.mp4
Future Work / Enhancement Ideas
✔️ Checklist
Signed-off-byline in the message. (more info)