Carousel - Next button is displayed when opening last image in report

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.56-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action performed:

1. Send a public image using markdown. You can use the following markdown: holla ![off]https://images.unsplash.com/photo-1705467648979-f66c9584827a?q=80&w=2000&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D
2. Click on '+' > Add attachment > send an image
3. Repeat step 1
4. Open the last sent image

Expected Result:

Previous button should be displayed on Carousel

Actual Result:

Next button is displayed on Carousel

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6423388_1711129084645.Screen_Recording_2024-03-22_at_8.37.03_in_the_evening.mp4

View all open jobs on GitHub

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @hayata-suenaga (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@hayata-suenaga FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[situchan]

Not blocker.
markdown image is new feature and still WIP of polish.
And this bug is known and planned to fix as follow-up.

[situchan]

cc: @kidroca

[hayata-suenaga]

@kidroca please feel free to close this issue if this regression is already tracked somewhere 😄

let me know if you don't have a permission to close this issue 👍

[situchan]

or maybe we can use this as tracking issue 😄

[hayata-suenaga]

okay I'll keep this as the tracking issue. Please link any PR that addresses this regressions once it's ready 🙇

[hayata-suenaga]

@situchan, please let me know if there is a project that is tracking the markdown feature implementation.

[melvin-bot]

@hayata-suenaga Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[hayata-suenaga]

changing the priority to weekly

[hayata-suenaga]

this issue is used as a tracking issue for a regression for the ongoing markdown project

[hayata-suenaga]

same as above

[hayata-suenaga]

@situchan, please update on this when you come back from OOO 🙇

[hayata-suenaga]

@situchan, I know you just came back from OOO but please provide an update on this issue when you have time 🙇

[hayata-suenaga]

Waiting for @situchan's response 😄

[situchan]

Since I was OOO for a while, I am not sure if this issue is being handled elsewhere.
Maybe @kidroca can answer

[kidroca]

Hi team,

This is actually intended behavior. The attachment carousel removes duplicate attachments when <img> tags have the same src. Tapping on a thumbnail shared multiple times opens the carousel at the first instance, hence the next arrow is visible. This situation occurs when public images are attached using the Markdown image syntax, allowing users to attach the same source multiple times. Instead of displaying multiple instances of the same image, we only show the first one in the carousel.

Even if we didn't remove duplicates, we lack the information to detect which instance of the repeating image was opened. When tapping a thumbnail, the carousel searches for an attachment matching the source of the thumbnail. Since duplicates share the same source, the carousel opens at the first match.

There were some issues that arose after implementing the Markdown image syntax feature, but they were considered minor and perhaps not affecting real use cases. As David stated:

"The others I think I'd like to experience before we bother solving."
Slack thread link

I haven't done any more work around Markdown images since then.

To address this, we can consider the following:

The ImageRenderer component, which renders thumbnails, has access to:

• Full image source (currently used to differentiate attachments)
• Report action ID (the report action containing the image)
• tnode (element information provided by the HTML parser)

We can update the ImageRenderer and related logic to also use reportActionID to uniquely identify attachments. To fully differentiate between attachments from the same action, we would need to use tnode information like index. These additional parameters would be forwarded along with the source to the Attachment Carousel to present the correct attachment.

However, we can't rely solely on tnode information. For example, the index doesn't match the element index identified in the Attachment Carousel, as the Carousel uses a simpler parser and skips plain text not wrapped by HTML tags.

We need someone to find a way to uniquely identify attachments. I've explored one path using available information, but someone else might try a different approach, such as modifying existing logic to pass more information, or double-checking my idea to identify any missed details.

[hayata-suenaga]

@kidroca, than you for the detailed explanation!

Tapping on a thumbnail shared multiple times opens the carousel at the first instance, hence the next arrow is visible.

This explains the issue!

There were some issues that arose after implementing the Markdown image syntax feature, but they were considered minor and perhaps not affecting real use cases.

I agree with this and this issue can also be considered as a minor issue. Let's close this and if the real user really gets confused by this, we can always reopen this issue to fix it 😄