Melkeydev / astrostation

https://astrostation.me
MIT License
209 stars 42 forks source link

roy/add hacker news #227

Closed royanger closed 8 months ago

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
astrostation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2023 8:23pm
eraychumak commented 1 year ago

Noticed hacker news doesn't get affected by the reset widget positions button on the side nav by the way. Something to do with the useSetDefault.tsx hook file I think.

royanger commented 1 year ago

Noticed hacker news doesn't get affected by the reset widget positions button on the side nav by the way. Something to do with the useSetDefault.tsx hook file I think.

Thanks for the info. Its definitely still a WIP so I haven't fully tested yet.

royanger commented 1 year ago

As a more general note, this is a WIP and was pushed up just so Melkey and I could talk about something else. I have at least one more commit locally and will be doing one or two beyond that before this is ready for proper review. That said, let me address the point you raised.

I see the default story we're going after is "Top", but I don't see any particular mention of that implementation in their repo README. I believe we'd first need to collect the IDs of https://hacker-news.firebaseio.com/v0/topstories.json?print=pretty

TOP/BEXT/NEW/ASK/SHOW/JOBS are 'state', or what was either selected by default or what the user selected. In the object on lines 11 to 49 that is used as the key, while the name field is for the buttons, the title for the widget title and apiSrc is used on line 60 for the fetch. I do need to test if the ?print=pretty is relevant or not for the fetch. There is some sort of bug with the store/current selection but I haven't looked at that yet. Its on the list along with a few other things to check and test.

parse for a random (or first, doesn't matter much) ID, then request again using that ID via https://hacker-news.firebaseio.com/v0/item/35191706.json?print=pretty.

The widget will use the 10 most recent items from the selected category. I grab that slice on line 62.

then request again using that ID via https://hacker-news.firebaseio.com/v0/item/35191706.json?print=pretty.

Right now the individual story details are fetched on lines 70 TO 83, though that part of the widget is completely rewritten in my local copy.

Unless I'm missing something (which I totally could), it seems a bit broken.

There are definitely issues with it, but it is still a WIP. Definitely some bugs to sort out.

mastermajorman commented 1 year ago

As a more general note, this is a WIP and was pushed up just so Melkey and I could talk about something else. I have at least one more commit locally and will be doing one or two beyond that before this is ready for proper review. That said, let me address the point you raised.

I see the default story we're going after is "Top", but I don't see any particular mention of that implementation in their repo README. I believe we'd first need to collect the IDs of https://hacker-news.firebaseio.com/v0/topstories.json?print=pretty

TOP/BEXT/NEW/ASK/SHOW/JOBS are 'state', or what was either selected by default or what the user selected. In the object on lines 11 to 49 that is used as the key, while the name field is for the buttons, the title for the widget title and apiSrc is used on line 60 for the fetch. I do need to test if the ?print=pretty is relevant or not for the fetch. There is some sort of bug with the store/current selection but I haven't looked at that yet. Its on the list along with a few other things to check and test.

parse for a random (or first, doesn't matter much) ID, then request again using that ID via https://hacker-news.firebaseio.com/v0/item/35191706.json?print=pretty.

The widget will use the 10 most recent items from the selected category. I grab that slice on line 62.

then request again using that ID via https://hacker-news.firebaseio.com/v0/item/35191706.json?print=pretty.

Right now the individual story details are fetched on lines 70 TO 83, though that part of the widget is completely rewritten in my local copy.

Unless I'm missing something (which I totally could), it seems a bit broken.

There are definitely issues with it, but it is still a WIP. Definitely some bugs to sort out.

Ahh, gotcha. I only took a quick peek at the outgoing req and judged based on that, mb. Looks good so far.

Melkeydev commented 1 year ago

There are some issues I am seeing:

When I am dragging it, it is possibly going outside the grid image

When clicking on the article link - nothing happens. It just changes the url of localhost. Can't actually see/read article. image

Moving any other widget (and this one included) is causing a refetch of state. image

Melkeydev commented 1 year ago

also there is a conflict in homepage.tsx

royanger commented 1 year ago

Moving any other widget (and this one included) is causing a refetch of state.

All components re-render, which is what's happening here. There is no refetch of the data. Put a console log of console.log('spotify') inside of Player.tsx and the same thing happens.

Melkeydev commented 1 year ago

@royanger Any update on this PR?

royanger commented 1 year ago

Right click on tasks appears to be broken. I'll have to sort that out. Otherwise I think this is ready. Feel free to review the PR, but don't merge until I look into the right click stuff.

eraychumak commented 1 year ago
  1. Clicking on each story leads to a 404.

  2. On first visit, the widget gets overflown (however after dragging it and then trying to make it overflow, it works as expected and does not allow you to make it overflow):

image

royanger commented 1 year ago
  1. Clicking on each story leads to a 404.

Somehow I reverted the links. Thanks for that catch. Fix pushed.

  1. On first visit, the widget gets overflown (however after dragging it and then trying to make it overflow, it works as expected and does not allow you to make it overflow):

This is not specific to this widget -- the same behaviour happens with the Twitch widget. IE, enable and toggle on all widgets with a wider browser window, close that tab, t hen revisit with a narrower window and it can be off screen. In the attached screen cap the Pomodoro clock, Quotes, Task Tracker, Twitch and Hacker News are all off screen. I'll open a separate issue for this, but this bug shouldn't stop this PR.

image

eraychumak commented 1 year ago

I'll open a separate issue for this, but this bug shouldn't stop this PR. @royanger yeah fair enough, I agree