Skip to content

Draft: Add 'copy url' buttons to 'game gif' and 'position gif' hyperlinks #15222

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
Merged
merged 4 commits into from
May 5, 2024
Merged

Draft: Add 'copy url' buttons to 'game gif' and 'position gif' hyperlinks #15222

merged 4 commits into from
May 5, 2024

Conversation

@Paul-Etienne
Copy link
Contributor

@Paul-Etienne Paul-Etienne commented May 4, 2024 •
edited
Loading

This PR adresses the issue #15165.

It adds two copy buttons to "Game gif" and "Screenshot position gif".

Edit: Any idea how to make the Build server check pass?

@Paul-Etienne Paul-Etienne force-pushed the feat/15165/add-copy-buttons-gif branch from 2017b1d to 5b6d342 Compare May 4, 2024 20:06
@Paul-Etienne
Copy link
Contributor Author

Paul-Etienne commented May 4, 2024

Here's what it looks like:

image

@brollin
Copy link
Collaborator

brollin commented May 5, 2024 •
edited
Loading

Nice work. Design-wise, I wonder if we can't improve things a little bit. From your screenshot, at a glance it's not clear if the first clipboard button is associated with the first link or the second. Somehow visually associating the two would be good.

I'm also wondering if on mobile we show share buttons, and if not mobile show copy to clipboard buttons 🤔

Finally, want to make this PR a draft PR for now until things are ready for merging?

ornicar added 2 commits May 5, 2024 09:28
@ornicar
Merge branch 'master' into feat/15165/add-copy-buttons-gif
809e683 
* master: (359 commits)
  scala tweak
  use link[data-css-key] to avoid regexes
  unused context naming
  scala tweaks while reading code
  New Crowdin updates (lichess-org#15203)
  Only test for 11 random export options
  scalafmtAll
  Add TreeBuilder Json tests
  Fix NewTree.PartitionNodeJsonWriter
  Fix NewTree Json writers
  makeMainlineWriter's argument is always false
  Use contextual abstraction for creating JsonWrites
  Add TreeBuilder tests and fix NewTreeBuilder
  Refactor AnnotatorTest
  Better import
  Implement NewTreeBuilder
  Fix annotanoter tests after changing Info max line
  Fix timeout from the mod public chat page
  fix board size slider lichess-org#15199
  prettier
  ...
@ornicar
use clipboard icon for copy buttons
d7fdee1
@ornicar
Copy link
Collaborator

ornicar commented May 5, 2024

making this right requires rewriting all the copyable links of the website.

which is what I'm doing now.

@Paul-Etienne
Copy link
Contributor Author

Paul-Etienne commented May 5, 2024 •
edited
Loading

By "making this right" you mean visually, as a response to brollin? Or are there copyable links throughout the website that were done differently maybe?
About more clearly associating the links with the buttons, maybe simply increasing the margin between the two groups of components would suffice?

I really like the idea of a share button on mobile. That would open the bottom sheet and allow the user to select a message app to send the link through right? I assume that's what you had in mind.

@Paul-Etienne Paul-Etienne changed the title Add 'copy url' buttons to 'game gif' and 'position gif' hyperlinks Draft: Add 'copy url' buttons to 'game gif' and 'position gif' hyperlinks May 5, 2024
@ornicar ornicar merged commit a0c1997 into lichess-org:master May 5, 2024
@Paul-Etienne Paul-Etienne deleted the feat/15165/add-copy-buttons-gif branch May 6, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Paul-Etienne @brollin @ornicar