#121 [feature] Carousel navigation and post text at right

[Anonymous]

Originally created by: Quimisagi

This PR finlly introduces the ability to navigate through carousels (using h/l). A small text below the media displays the current index in the carousel. Furthermore, by pressing o, the current media will be opened in the browser.

https://github.com/user-attachments/assets/4b9e3493-fb8f-4eb8-8a45-02cc4b32a20d

Also changed the feed layout so it displays post text at the right of the media (takes around 50% of the space)

With this I believe that the most important features regarding the feed are implemented, though it still may have room for improvements.

Notes

• Will eventually integrate the Image component introduced on [#104]
• Those new features were only applied to the ListMediaViewer and are not included in TimelineMediaViewer. My apply that to TimelineMediaViewer eventually if there's some demand for it.

[Anonymous]

Originally posted by: supreme-gg-gg

Super cool new feature, will test soon but make sure you're merging into ts-migration/main instead of main 😄

[Anonymous]

Originally posted by: Quimisagi

Super cool new feature, will test soon but make sure you're merging into ts-migration/main instead of main 😄

Right
Sorry, I forgot, lol
Will do it later

[Anonymous]

Originally posted by: Adventune

Would love to have this done to be reused for posting (#119).

Try to render actual images instead of ASCII.

What comes to this: I am planning to within the same issue (#119) update the image rendering. terminal-image package seems much better suited. It renders using half blocks for terminals that do not support images and just renders the image if the terminal does support it.

[Anonymous]

Originally posted by: Quimisagi

Would love to have this done to be reused for posting (#119).

Try to render actual images instead of ASCII.

What comes to this: I am planning to within the same issue (#119) update the image rendering. terminal-image package seems much better suited. It renders using half blocks for terminals that do not support images and just renders the image if the terminal does support it.

Yeah, I agree that terminal-image definitely would work better!
It's not yet done but I think it's indeed the way to go

[Anonymous]

Originally posted by: supreme-gg-gg

Just to let you guys know that @endernoke is working on a package / utility to support image display in most modern emulators (I believe based on terminal-image and some other protocols?) and is tracked in [#104]

[Anonymous]

Originally posted by: Adventune

Just to let you guys know that @endernoke is working on a package / utility to support image display in most modern emulators (I believe based on terminal-image and some other protocols?) and is tracked in [#104]

Alright. That should be fine then. I think an image/carousel component could be in place. The image rendering seems to be currently left to whatever View/Component wants to use an image. Thinking of a component that takes source (single to multiple, uri or filepath) and renders the image/carousel. Maybe with some style arguments (width, height).

The image rendering work seems to be quite scattered across multiple features/issues (atleast as I am stepping in to the project), so could some combined tracking issue be made? @supreme-gg-gg

[Anonymous]

Originally posted by: Quimisagi

Just to let you guys know that @endernoke is working on a package / utility to support image display in most modern emulators (I believe based on terminal-image and some other protocols?) and is tracked in [#104]

Alright. That should be fine then. I think an image/carousel component could be in place. The image rendering seems to be currently left to whatever View/Component wants to use an image. Thinking of a component that takes source (single to multiple, uri or filepath) and renders the image/carousel. Maybe with some style arguments (width, height).

The image rendering work seems to be quite scattered across multiple features/issues (atleast as I am stepping in to the project), so could some combined tracking issue be made? @supreme-gg-gg

I have already implemented an ASCII rendering for the feed, and I was planning to start working on the implementation of the image rendering too, but looks like @endernoke is already handling it, I'll just wait for that to integrate it to the feed later :P
Currently the ASCII renderer only takes the URL of the image, and it fetches it and adjusts its size based on the terminal size.

[Anonymous]

Originally posted by: endernoke

Just to let you guys know that @endernoke is working on a package / utility to support image display in most modern emulators (I believe based on terminal-image and some other protocols?) and is tracked in [#104]

Alright. That should be fine then. I think an image/carousel component could be in place. The image rendering seems to be currently left to whatever View/Component wants to use an image. Thinking of a component that takes source (single to multiple, uri or filepath) and renders the image/carousel. Maybe with some style arguments (width, height).
The image rendering work seems to be quite scattered across multiple features/issues (atleast as I am stepping in to the project), so could some combined tracking issue be made? @supreme-gg-gg

I have already implemented an ASCII rendering for the feed, and I was planning to start working on the implementation of the image rendering too, but looks like @endernoke is already handling it, I'll just wait for that to integrate it to the feed later :P Currently the ASCII renderer only takes the URL of the image, and it fetches it and adjusts its size based on the terminal size.

What about we build a generic Ink component responsible for rendering an image (like ink-image but much more powerful), then we can wrap the media view / posts / carousel around it, so we can modularize the code and abstract away the rendering and protocol implementation details.

As @Adventune said the component might take source image and styling props, I think it should accept the following parameters:

• src: image URL or filepath
• width, height in terms of terminal cells or pixels
• protocol: how the image would be rendered, e.g. with ASCII or half blocks or sixel or whatever

@Quimisagi @Adventune what do you think?

If you guys are positive about this then we can start by refactoring the ASCII rendering into a standalone component and implement more rendering strategies incrementally.

[Anonymous]

Originally posted by: endernoke

Let's track all image rendering related progress in [#104]

The terminal-image library should be a good starting point

[Anonymous]

Originally posted by: Adventune

@endernoke That sounds great! Would make it possible to use the component while it is improved over time. The props sound good (src is the only one I would make required).

[Anonymous]

Originally posted by: supreme-gg-gg

@Quimisagi is this ready for review?

[Anonymous]

Originally posted by: Quimisagi

@Quimisagi is this ready for review?

Wanted to solve the issue of media not longer automatically render itself when its ready...

[Anonymous]

Originally posted by: Quimisagi

@Quimisagi is this ready for review?

Ok, now it's ready!

[Anonymous]

Originally posted by: Quimisagi

Looks like the feed doesnt work anymore after merging the changes in ts-migration/main....can someone please confirm?

[Anonymous]

Originally posted by: endernoke

Looks like the feed doesnt work anymore after merging the changes in ts-migration/main....can someone please confirm?

I got the following error when testing:

$ npm run start feed

> instagram-ts@0.0.0 start
> node dist/cli.js feed

Warning: React has detected a change in the order of Hooks called by ListMediaDisplay. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

   Previous render            Next render
   ------------------------------------------------------

1. useState                   useState
2. useState                   useState
3. useState                   useState
4. undefined                  useContext
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    at ListMediaDisplay (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/components/ListMediaDisplay.js:4:44)
    at MediaView (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/views/MediaView.js:6:37)
    at Feed (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/commands/feed.js:16:32)
    at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/pastel/build/_app.js:2:31)
    at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/ink/build/components/App.js:17:16)

  ERROR  Rendered more hooks than during the previous render.

 node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13

 6858:   } else {
 6859:     // Clone from the current hook.
 6860:     if (nextCurrentHook === null) {
 6861:       throw new Error('Rendered more hooks than during the previous render.');
 6862:     }
 6863:
 6864:     currentHook = nextCurrentHook;


 - updateWorkInProgressHook (node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13)
 - updateEffectImpl (node_modules/react-reconciler/cjs/react-reconciler.development.js:7421:14)
 - updateEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:7452:10)
 - Object.useEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:8216:14)
 - useEffect (node_modules/react/cjs/react.development.js:1634:21)
 - useInput (node_modules/ink/build/hooks/use-input.js:32:5)
 - ListMediaDisplay (dist/ui/components/ListMediaDisplay.js:54:5)
 - renderWithHooks (node_modules/react-reconciler/cjs/react-reconciler.development.js:6659:18)
 - updateFunctionComponent (node_modules/react-reconciler/cjs/react-reconciler.development.js:10790:20)
 - beginWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:12813:16)
 - beginWork$1 (node_modules/react-reconciler/cjs/react-reconciler.development.js:19608:14)
 - performUnitOfWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:18742:12)
 - workLoopSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18648:5)
 - renderRootSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18616:7)
 - performSyncWorkOnRoot (node_modules/react-reconciler/cjs/react-reconciler.development.js:18232:20)
 - flushSyncCallbacks (node_modules/react-reconciler/cjs/react-reconciler.development.js:2936:22)
 - flushSyncCallbacksOnlyInLegacyMode (node_modules/react-reconciler/cjs/react-reconciler.development.js:2915:5)
 - scheduleUpdateOnFiber (node_modules/react-reconciler/cjs/react-reconciler.development.js:17685:7)
 - dispatchSetState (node_modules/react-reconciler/cjs/react-reconciler.development.js:7881:7)
 - fetchFeed (dist/commands/feed.js:34:21)

@Quimisagi Is this the same error you got

[Anonymous]

Originally posted by: Quimisagi

Looks like the feed doesnt work anymore after merging the changes in ts-migration/main....can someone please confirm?

I got the following error when testing:
```
$ npm run start feed

instagram-ts@0.0.0 start
node dist/cli.js feed

Warning: React has detected a change in the order of Hooks called by ListMediaDisplay. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

Previous render Next render

---

1. useState useState
2. useState useState
3. useState useState

4. undefined useContext
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   at ListMediaDisplay (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/components/ListMediaDisplay.js:4:44)
   at MediaView (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/views/MediaView.js:6:37)
   at Feed (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/commands/feed.js:16:32)
   at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/pastel/build/_app.js:2:31)
   at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/ink/build/components/App.js:17:16)

ERROR Rendered more hooks than during the previous render.

node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13

6858: } else {
6859: // Clone from the current hook.
6860: if (nextCurrentHook === null) {
6861: throw new Error('Rendered more hooks than during the previous render.');
6862: }
6863:
6864: currentHook = nextCurrentHook;

• updateWorkInProgressHook (node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13)
• updateEffectImpl (node_modules/react-reconciler/cjs/react-reconciler.development.js:7421:14)
• updateEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:7452:10)
• Object.useEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:8216:14)
• useEffect (node_modules/react/cjs/react.development.js:1634:21)
• useInput (node_modules/ink/build/hooks/use-input.js:32:5)
• ListMediaDisplay (dist/ui/components/ListMediaDisplay.js:54:5)
• renderWithHooks (node_modules/react-reconciler/cjs/react-reconciler.development.js:6659:18)
• updateFunctionComponent (node_modules/react-reconciler/cjs/react-reconciler.development.js:10790:20)
• beginWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:12813:16)
• beginWork$1 (node_modules/react-reconciler/cjs/react-reconciler.development.js:19608:14)
• performUnitOfWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:18742:12)
• workLoopSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18648:5)
• renderRootSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18616:7)
• performSyncWorkOnRoot (node_modules/react-reconciler/cjs/react-reconciler.development.js:18232:20)
• flushSyncCallbacks (node_modules/react-reconciler/cjs/react-reconciler.development.js:2936:22)
• flushSyncCallbacksOnlyInLegacyMode (node_modules/react-reconciler/cjs/react-reconciler.development.js:2915:5)
• scheduleUpdateOnFiber (node_modules/react-reconciler/cjs/react-reconciler.development.js:17685:7)
• dispatchSetState (node_modules/react-reconciler/cjs/react-reconciler.development.js:7881:7)
• fetchFeed (dist/commands/feed.js:34:21)

```

@Quimisagi Is this the same error you got

Yes, it's exactly the same!

[Anonymous]

Originally posted by: endernoke

Looks like the feed doesnt work anymore after merging the changes in ts-migration/main....can someone please confirm?

I got the following error when testing:
```
$ npm run start feed

instagram-ts@0.0.0 start
node dist/cli.js feed

Warning: React has detected a change in the order of Hooks called by ListMediaDisplay. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

Previous render Next render

---

1. useState useState
2. useState useState
3. useState useState

4. undefined useContext
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   at ListMediaDisplay (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/components/ListMediaDisplay.js:4:44)
   at MediaView (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/ui/views/MediaView.js:6:37)
   at Feed (file:///home/endernoke/repos/instagram-cli/instagram-ts/dist/commands/feed.js:16:32)
   at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/pastel/build/_app.js:2:31)
   at App (file:///home/endernoke/repos/instagram-cli/instagram-ts/node_modules/ink/build/components/App.js:17:16)

ERROR Rendered more hooks than during the previous render.

node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13

6858: } else {
6859: // Clone from the current hook.
6860: if (nextCurrentHook === null) {
6861: throw new Error('Rendered more hooks than during the previous render.');
6862: }
6863:
6864: currentHook = nextCurrentHook;

• updateWorkInProgressHook (node_modules/react-reconciler/cjs/react-reconciler.development.js:6861:13)
• updateEffectImpl (node_modules/react-reconciler/cjs/react-reconciler.development.js:7421:14)
• updateEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:7452:10)
• Object.useEffect (node_modules/react-reconciler/cjs/react-reconciler.development.js:8216:14)
• useEffect (node_modules/react/cjs/react.development.js:1634:21)
• useInput (node_modules/ink/build/hooks/use-input.js:32:5)
• ListMediaDisplay (dist/ui/components/ListMediaDisplay.js:54:5)
• renderWithHooks (node_modules/react-reconciler/cjs/react-reconciler.development.js:6659:18)
• updateFunctionComponent (node_modules/react-reconciler/cjs/react-reconciler.development.js:10790:20)
• beginWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:12813:16)
• beginWork$1 (node_modules/react-reconciler/cjs/react-reconciler.development.js:19608:14)
• performUnitOfWork (node_modules/react-reconciler/cjs/react-reconciler.development.js:18742:12)
• workLoopSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18648:5)
• renderRootSync (node_modules/react-reconciler/cjs/react-reconciler.development.js:18616:7)
• performSyncWorkOnRoot (node_modules/react-reconciler/cjs/react-reconciler.development.js:18232:20)
• flushSyncCallbacks (node_modules/react-reconciler/cjs/react-reconciler.development.js:2936:22)
• flushSyncCallbacksOnlyInLegacyMode (node_modules/react-reconciler/cjs/react-reconciler.development.js:2915:5)
• scheduleUpdateOnFiber (node_modules/react-reconciler/cjs/react-reconciler.development.js:17685:7)
• dispatchSetState (node_modules/react-reconciler/cjs/react-reconciler.development.js:7881:7)
• fetchFeed (dist/commands/feed.js:34:21)
  ```

@Quimisagi Is this the same error you got

Yes, it's exactly the same!

See my review comment above, it should fix it 😄

[Anonymous]

Originally posted by: Quimisagi

See my review comment above, it should fix it 😄

That definitely fixed the error but still something changed after the merge with ts-migration/main
There's no errors now, but looks like isLoading becomes false even before data has finished loading making it display an empty media view for a moment.
It wasn't happening before so I think it has to do with the changes in useInsagramClient.js

[Anonymous]

Originally posted by: Quimisagi

Ok, I think it may be ready now anyway ;D

[Anonymous]

Originally posted by: Quimisagi

There are some flickering when moving between posts and carousel is this happening for others too? Not a big deal tho since it doesn't happen if there is no user input

I think it also happens to me but haven't figured out the cause yet.

[Anonymous]

Originally posted by: supreme-gg-gg

There are some flickering when moving between posts and carousel is this happening for others too? Not a big deal tho since it doesn't happen if there is no user input

I think it also happens to me but haven't figured out the cause yet.

I can only reproduce this on iTerm2 lol, it works fine for example on Kitty, seems to be a quite terminal specific issue. I think we can merge it first since it doesn't really affect UX and it fixes the react hook issue above

[Anonymous]

Ticket changed by: supreme-gg-gg

• status: open --> closed