Skip to content

lightbox: Use ZulipIcons.copy for copy-link button #2016

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.

Sign up for GitHub

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

Jump to bottom
Open
sarafarajnasardi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

lightbox: Use ZulipIcons.copy for copy-link button #2016

sarafarajnasardi wants to merge 1 commit into from

Conversation

@sarafarajnasardi
Copy link

@sarafarajnasardi sarafarajnasardi commented Dec 3, 2025

Fixed #2014
Before
b3b94a41-edc1-4865-a40b-e94dcacaecab

After
4e81ac5c-eec2-4c8a-ab50-035f298a8ac3

@alya
Copy link
Collaborator

alya commented Dec 4, 2025

The issue is for the lightbox, and the screenshots are not of the lightbox.

@sarafarajnasardi
Copy link
Author

sarafarajnasardi commented Dec 4, 2025

Hi @alya , just to clarify the screenshot I shared earlier was taken inside the lightbox.
The image displayed in the lightbox happened to be a screenshot of the message action sheet, which may have caused confusion.
The UI shown around the image (black fullscreen viewer) is the actual lightbox UI where the copy icon is used.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Dec 4, 2025

Thanks, looks good!

Commit-message nits (see doc):

  • Capitalize the part after "lightbox:", so "Use […]" instead of "use […]"
  • Say Fixes #2014 in the commit-message body

@chrisbobbe chrisbobbe requested a review from gnprice December 4, 2025 18:32
@chrisbobbe chrisbobbe added the integration review Added by maintainers when PR may be ready for integration label Dec 4, 2025
@sarafarajnasardi
lightbox: Use Zulip copy icon for "Copy link" button
4c3e4fb 
Replace the generic Icons.copy with ZulipIcons.copy for the
"Copy link" button in the lightbox UI. This matches the icon
used in the message action sheet's "Copy message text" button
and follows our Figma designs.

Fixes: zulip#2014
@sarafarajnasardi sarafarajnasardi force-pushed the use-zulip-icon-for-lightbox-copy branch from c8b69f3 to 4c3e4fb Compare December 4, 2025 18:52
@sarafarajnasardi
Copy link
Author

sarafarajnasardi commented Dec 4, 2025

Thanks for the review!
I’ve updated the commit message with the capitalization fix and added “Fixes #2014” in the body as suggested.

@alya
Copy link
Collaborator

alya commented Dec 4, 2025

Ohh, yes, I was confused by the message action sheet.

@sarafarajnasardi sarafarajnasardi changed the title lightbox: use ZulipIcons.copy for copy-link button lightbox: Use ZulipIcons.copy for copy-link button Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gnprice gnprice Awaiting requested review from gnprice

Assignees

@gnprice gnprice

Labels

integration review Added by maintainers when PR may be ready for integration

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

lightbox: Use ZulipIcons.copy for the "Copy link" button, not Icons.copy

4 participants

@sarafarajnasardi @alya @chrisbobbe @gnprice