Differential D25301

Diffusion: add "Permalink" button
Needs ReviewPublic
Actions

Authored by valerio.bozzolan on Jun 16 2023, 23:09.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15256: Diffusion file: add a way to copy permalink

Summary

The intent of this change is to allow people to have such lovely conversation
and that they make sense over time even after subsequent commit:

A: where the hell is the logic of this?!?
B: yeah look here <permalink> line 420

After this change, having that "<permalink>" is easier thanks to the "Permalink" button
that just generates the canonical URL with the stable commit hash:

Diffusion after D25301 highlighted with GIMP green peppers.png (763×994 px, 142 KB)

From there, you can highlight line 420, being sure that your quote will always remain correct.

To clarify, if you are visiting this page:

https://we.phorge.it/source/phorge/browse/master/webroot/rsrc/css/application/base/phui-theme.css

This would be the destination of the Permalink button:

https://we.phorge.it/source/phorge/browse/master/webroot/rsrc/css/application/base/phui-theme.css;ad01e26af71e7d7936dd2347eca47674255a7838

Note that other platforms have such button. For example, see the "Permalink" button
on this random GitLab repository:

https://gitlab.com/ItalianLinuxSociety/linux.it/-/blob/master/_config.yml

There is still some room for contribution to make the URL change without reloading
the page. Any follow-up patch is welcome.

Closes T15256

Test Plan

Browse a Diffusion file: should see a "Permalink" button.

Visit that button: now it's marked as selected.

Browse a Diffusion directory: it should have not that button.

Diff Detail

Repository

rP Phorge

Branch

diffusion-permalink-buttonzzzzzz

Lint

Lint Passed

Severity	Location	Code	Message
Advice	src/applications/diffusion/controller/DiffusionBrowseController.php:630	XHP16	TODO Comment

Unit

Tests Passed

Build Status

Buildable 570
Build 570: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Jun 16 2023, 23:09

Owners added a reviewer: O1: Blessed Committers.Jun 16 2023, 23:09

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptJun 16 2023, 23:09

valerio.bozzolan requested review of this revision.Jun 16 2023, 23:09

Harbormaster completed remote builds in B570: Diff 979.Jun 16 2023, 23:09

valerio.bozzolan edited the test plan for this revision. (Show Details)Jun 16 2023, 23:11

valerio.bozzolan edited the summary of this revision. (Show Details)Jun 16 2023, 23:22

valerio.bozzolan edited the summary of this revision. (Show Details)Jun 17 2023, 22:17

valerio.bozzolan edited the summary of this revision. (Show Details)Jun 27 2023, 12:10

valerio.bozzolan mentioned this in T15256: Diffusion file: add a way to copy permalink.Sep 26 2023, 06:58

This basically works

I’m not sure about Permalink being a top-level button. Maybe under the View Options menu?

I’m not sure about Permalink being a top-level button. Maybe under the View Options menu?

I explored a bit. It seems View Options is boldly related to "changing a small portion of the boxed file View". Instead the top buttons are related to "interacting with the whole page".

I think this explains the Raw file button that is a top link instead.

I'd prefer not to expose this as an always-visible button simply because it takes screen estate on smaller screens.
Maybe an entry Copy Revision Permalink between two separators in the expandable "View Options" dropdown is an option?

In any case, such functionality ideally actually copies the Permalink to the clipboard. There is some existing code in https://we.phorge.it/source/phorge/browse/master/src/applications/uiexample/examples/PHUIButtonExample.php$109-121 , has that been explored?