Summary:
In edit mode, Archived Panels are shown differently,
with content text in grey and with the usual red tag.
In view mode, Archived Panels are just not rendered.
This makes it easier to design Dashboards with individual
components that can be de-activated and re-activated on the fly.
Closes T15366.
{F325637}
Test Plan:
Have a Dashboard somewhere (e.g in your homepage).
Visit a Panel (W-ID) and Archive it.
See that now the Panel is not shown anymore on the Dashboard.
Edit the Dashboard and you see the Panel again as "Archived".
Anything around is just normal.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15366
Differential Revision: https://we.phorge.it/D25368
Summary:
The current hovercard of a wiki document has no further information except the title. This commit adds object type, project tags, parent documents, last author and last edited time to the card.
Preview:
{F313614}
Preview in a pessimistic case:
{F325478}
Closes T15433
Test Plan: Edit a wiki document with/without project tags and parent documents and see the hovercard in the feed.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey, Cigaryno
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey, Cigaryno
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15433
Differential Revision: https://we.phorge.it/D25303
Summary:
Firefox does not ellipsize long filenames after selecting them in the File Upload dialog; Chromium does (for unknown reasons).
Could hardcode the "remaining" width for the `<input>` element itself (approx. 340px) based on calculating the CSS widths and margins of all surrounding elements but that is error-prone if CSS for one of those surrounding elements ever changed.
Thus instead use `max-width: stretch` for the `<input>` element itself. Per its limited support (see https://caniuse.com/?search=max-width), set also `-moz-available` (https://bugzilla.mozilla.org/show_bug.cgi?id=1495868) and `-webkit-fill-available` (see https://bugs.chromium.org/p/chromium/issues/detail?id=611857) aliases.
Closes T15553
Test Plan:
* Both in Firefox and in Chromium, go to an existing task, select "File Upload", select a file with a long name.
* See that the filename is now correctly ellipsized in Firefox.
* See no changes in Chromium.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15553
Differential Revision: https://we.phorge.it/D25347
Summary:
In a Ponder Answer, when adding a Comment, the textarea indentation was a
a little bit unintuitive.
After this change, the textarea is aligned as a Comment:
| Before | After |
|-----------|-----------|
| {F286220} | {F286221} |
Other changes are welcome but this seems to me a good minimum.
I also hope this could help not to confuse the Comment field with the Answer field.
Closes T15350
Test Plan:
- create a Ponder Question
- create an Answer
- look at the Comments input field that now should be more nicely indented
- the normal Response field is not changed at all
- the mobile view is not changed at all
- click on "Pin Form On Screen" and it must be as before
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15350
Differential Revision: https://we.phorge.it/D25192
Summary:
Increase required height for avatar image
Patch provided by Stang in https://phabricator.wikimedia.org/T307492
Closes T15552
Test Plan:
* Set `/config/edit/user.require-real-name/` to `Make real names optional`
* Log in as a user without a real name set
* Open the user menu in the top bar and look at the user avatar image
Reviewers: O1 Blessed Committers, Cigaryno, avivey
Reviewed By: O1 Blessed Committers, Cigaryno, avivey
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15552
Differential Revision: https://we.phorge.it/D25348
Summary:
Add blur and sepia filter to disabled user profile image.
Before this change, the image was just greyed out but still fully recognizable.
Closes T15515
Test Plan:
* Create an account with a custom profile picture
* Disable the account
* Go to user profile of the account
* Go to a task with activity of that account and hover over the account to open card view
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15515
Differential Revision: https://we.phorge.it/D25322
Summary:
Before this change, if you search a file using Locate File,
this was the only accepted syntax for a full-path:
path/to/something.txt
After this change, some relative/absolute Unix-like variants
are also accepted:
./path/to/something.txt
/path/to/something.txt
Similar prefixes can be frequent when you quickly copy-paste things
from shell commands like 'grep' or 'find' etc.
Ref T15508
Test Plan:
Visit a lovely repository like Phorge and use Locate File with these:
1. src/applications/maniphest/editor/ManiphestEditEngine.php
2. /src/applications/maniphest/editor/ManiphestEditEngine.php
3. ./src/applications/maniphest/editor/ManiphestEditEngine.php
After this change, also 2. and 3. return the expected file.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15508
Differential Revision: https://we.phorge.it/D25320
Summary:
Have a consistent rendering experience between File page and file embedded in a task for PNG image files with an alpha channel:
Show a checkered background on the embedded image instead of a white background; show full contrast when hovering over the embedded image.
Closes T15473
Test Plan:
See steps in T15473 - compare an image file embedded in a task before and after applying this patch.
- test in Task description
- test in Task comment
- test in Task comment inside something weird like a table
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15473
Differential Revision: https://we.phorge.it/D25294
Summary:
On mobile devices like tablets or toasters the "Persistent Chat"
floating widget is already hidden.
So, the related checkbox available from the top navigation bar
is just confusing on tablet and mobile devices / toasters, since
that nice checkbox does nothing there.
On mobile and tablet, this is the graphical change:
| Before | After |
|----------|-----------|
|{F281239} | {F281235} |
This change do not change anything for desktop devices.
So, on desktop, that checkbox is obviously still visible.
Closes T15240
Test Plan:
- test on tablet and below: now the checkbox should be not visible
- test on desktop: the checkbox should still be visible
Reviewers: O1 Blessed Committers, Cigaryno, bfs, speck
Reviewed By: O1 Blessed Committers, Cigaryno, bfs, speck
Subscribers: avivey, bfs, dcog, chris, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15240
Differential Revision: https://we.phorge.it/D25120
Summary:
Removes the -khtml, -moz and -ms prefix, since most Browsers are natively supporting the user select directive.
The -webkit prefix is still kept or added for Safari, wich does not support user-select.
Ref: see https://we.phorge.it/D25024#815 for context
Test Plan: Removing the CSS should change nothing in modern browsers.
Reviewers: O1 Blessed Committers, speck, valerio.bozzolan, Matthew
Reviewed By: O1 Blessed Committers, speck, valerio.bozzolan
Subscribers: Cigaryno, Matthew, speck, tobiaswiese, valerio.bozzolan
Differential Revision: https://we.phorge.it/D25025
Summary:
This change fixes the following recent JavaScript error:
Error: Empty ID passed to JX.$()!
The regression was introduced here:
90f9da643d
Closes T15272
Test Plan: - with this change I was able to open Conpherence Persistent Chat and Drop a file
Reviewers: O1 Blessed Committers, Dylsss, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: Dylsss, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15272
Differential Revision: https://we.phorge.it/D25134
Summary:
Add a new RemarkupMetadata class and use with upload button as well as drag and drop pathways.
With this change, files are now attached automatically to the object. This this could solve
several issues, for example when a person uploads a file but only that author is able to see it.
Ref T15106
Test Plan:
- Drag and drop file, upload file with button. Check that both files are attached with "attachedFilePHIDs" values.
- drop file in a Task description (now works)
- drop file in a Task comment (now works)
- drop file in an edited Task comment (still not supported)
- This was already tested in Wikimedia Foundation: https://phabricator.wikimedia.org/D1203
Reviewers: O1 Blessed Committers, avivey, Matthew
Reviewed By: O1 Blessed Committers, avivey, Matthew
Subscribers: MCPCN, avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #remarkup
Maniphest Tasks: T15106
Differential Revision: https://we.phorge.it/D25052
Summary:
The left part of a Removed Comment can contains User Badges that,
after this change, are more consistent with the new general softness.
| Before | Proposed |
|------------|-----------|
| {F277371} | {F277373} |
Probably five people in a million will notice this change, BUT,
these people will have a huge, deep, breath of relief, knowing that
Phorge takes care about their obsessive-compulsive impulses, to
have everything nice and consistent and neat. You are welcome!
Closes T15235
Test Plan:
- do something to deserve a Badge
- share a Comment somewhere (Maniphest?)
- delete that comment
- check that the badge is slightly softer
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15235
Differential Revision: https://we.phorge.it/D25112
Summary:
This small restyle makes any Removed Comment a little less
prominent than normal ones, with the goal of decreasing a
bit your in-page distractions and increase your individual
productivity in your business by at least 250 milliseconds
every 48 hours of hard work in front of your monitor.
| Before | After |
|---------------------|---------------------|
| {F274834,size=full} | {F274835,size=full} |
This implementation (which is called "Kasper on Diet")
contains these specific changes for Removed Comments:
- user icon visibility: reduced by ~50% (-> Kasper)
- black "trash" icon: reduced by ~50% (-> Diet)
- texts: visibility reduced by ~50%
- vertical padding: reduced from 16px down to 4px
Note that if your Phorge is under the Serious Business Mode,
it seems it is still technically possible to manually
activate the "Decaying Curse" proposal mentioned in the Task.
Closes T15192
Test Plan:
- Add a Comment "I love Phorge"
- Add a Comment "I love Phabricator"
- Mark the second Comment as Removed
- Call a person at your desk
- Plug that person to an eyeball tracker
If the general attention focuses first on a normal Comment and then
on the Removed Comment, this change works perfectly.
Reviewers: O1 Blessed Committers, Cigaryno, avivey
Reviewed By: O1 Blessed Committers, Cigaryno, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Tags: #comments
Maniphest Tasks: T15192
Differential Revision: https://we.phorge.it/D25096
Summary:
Hide the "hidden" fields on custom form previews.
Before this change, Phabricator's custom form preview doesn't actually hide the
hidden fields, instead it shows them at 50% opacity. It looks cluttered and
doesn't provide a very useful "preview" at all.
This just hides some fields via CSS.
Cherry picked from:
https://phabricator.wikimedia.org/rPHABbba62cf5243538af9e37cc1211a01d247294f9f7
Upstream Task:
https://phabricator.wikimedia.org/T209743
Test Plan: Tested in Wikimedia's fork. I believe it results in better UX.
Reviewers: #blessed_committers, O1 Blessed Committers, Cigaryno, avivey, Matthew, valerio.bozzolan
Reviewed By: #blessed_committers, O1 Blessed Committers, Cigaryno, avivey, valerio.bozzolan
Subscribers: avivey, Cigaryno, speck, tobiaswiese, valerio.bozzolan, Matthew
Maniphest Tasks: T15081
Differential Revision: https://we.phorge.it/D25037
Summary:
This change contains a very minimal graphical change for Workboards
with a custom background.
https://we.phorge.it/T15186
After this change, if your Workboard has a custom background color,
the opacity of the floating menu of the Column is not 0.9 but 0.95,
so it's increased a bit. The problem with the previous value, is that
when you open the menu, you see too much text underneath, and this
could worsen the readability. Now also probably, but less I hope.
Closes T15186
Test Plan:
- Workboard > Change Background Color > Pick a nice color
- Backlog > Edit
- Enjoy the extra 0.05 of opacity of that menu
- NEVER ACCEPT IF YOUR EYES DETECT JUST A 0.04999998 OF EXTRA OPACITY
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15186
Differential Revision: https://we.phorge.it/D25088
Summary:
The CTRL+click now opens the links in a new tab, like any other
normal link opened with CTRL+click.
Note that the middle-click was already working.
Closes T15157
Test Plan:
- visit a Workboard
- column > menu > mouse on "Create Task"
- CTRL+click: open in new tab (→ now works)
- normal click: open the pop-up (→ still works)
- middle-click: open in new tab (→ still works)
- column > single Task > mouse on "Edit"
- CTRL+click: open in new tab (→ now works)
- normal click: open the pop-up (→ still works)
- middle-click: open in new tab (→ still works)
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15157
Differential Revision: https://we.phorge.it/D25072
Summary:
Rest assured: external links remain evil, by default.
Don't adopt them randomly by induction.
Whether you believe it or not, this specific external
link merited some deep thoughts on Phorge:
- https://we.phorge.it/T15172
So, whenever you use a mouse, a finger, or whenever we have
a confirmation dialog or not to prevent onblur disasters,
this change is probably consistent with common expectations.
Having said, external links remain evil - by default.
Closes T15172
Test Plan:
- Drag & Drop a File on a Remarkup text
- click on the link inside the popup
- it opens in a new tab (without risk of form loss)
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15172
Differential Revision: https://we.phorge.it/D25077
Summary:
This modification improves a single link shown to the user.
The goal is to make it easier to change file permissions or
file name or whatever just after the upload of the file itself.
Before this change, doing that was difficult because clicking
on the popup would send you back to the binary file only.
Closes T15165
Test Plan:
- upload a file using drag & drop (for example in a comment)
- click on the popup's link
- you see the File page, not the binary file
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15165
Differential Revision: https://we.phorge.it/D25074
Summary:
Fix a JavaScript regression encapsulating the problematic part into an `if`.
Other minor changes:
- dedicate a variable for the confirmation messages to improve i18n in the future (but also to avoid 80 characters and make lint happy)
- replace `confirm` with `window.confirm` (to make lint happy)
Ref T15034
Ref D25015
Test Plan:
- surf on your local Phorge
- no JavaScript errors in console
Reviewers: bekay, Ekubischta, O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15034
Differential Revision: https://we.phorge.it/D25076
Summary:
Fix middle-click, CTRL+click, right-click etc. on Typehead search results.
Closes T15149
Test Plan:
Try the following actions on various typeheads:
- right-click
- middle-click
- CTRL+click
- normal click
Demonstration video (2M) showing After patch (on localhost) and Before patch (here on we.phorge.it), where I middle-click and normal-click on menu entries:
{F256610}
Notes:
- the middle click now works (opening in new tab)
- the CTRL+click (or "command" key + click) now works (opening in new tab)
- the right click now opens the context menu (previously broken)
- the normal click should just click (as usual)
Try on:
- search results while typing in main search bar
- search results when editing a Task Tags / assigned to, etc.
- try to click on other weird places
- $$$
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15149
Differential Revision: https://we.phorge.it/D25069
Summary: This fully updates some login provider icons. Somehow this didn't work fully on previous updates. I used --force this time.
Test Plan: None just ran the script which regenerated the spritesheets.
Reviewers: O1 Blessed Committers, #blessed_committers, avivey
Reviewed By: O1 Blessed Committers, #blessed_committers, avivey
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #auth
Differential Revision: https://we.phorge.it/D25055
Summary: Marks Line Numbers in Diffusion File Preview as unselectable
Test Plan:
- Select multiple Lines from a File Preview in Diffusion
- Copy them into a Text Editor
- The Leading Tabs should no longer included
Reviewers: O1 Blessed Committers, speck, Ekubischta
Reviewed By: O1 Blessed Committers, speck, Ekubischta
Subscribers: Ekubischta, speck, tobiaswiese, valerio.bozzolan
Differential Revision: https://we.phorge.it/D25024
Summary: Honestly I did not realize that Differential can do this. Anyway this is related to T15034 ... Originally opened at https://secure.phabricator.com/T12676
Test Plan:
1) Start creating a task via a Workboard in Manifest, type many words, press `ESC`
2) Start creating a task via a Workboard in Manifest, type no words, press `ESC`
Reviewers: O1 Blessed Committers, Ekubischta, speck
Reviewed By: O1 Blessed Committers, Ekubischta, speck
Subscribers: Leon95, 20after4, avivey, Ekubischta, speck, tobiaswiese
Tags: #maniphest
Differential Revision: https://we.phorge.it/D25015
Summary:
The current default wordmark is "Phabricator" which is trademarked and the
default logo is also copyright.
(This change was made by @speck directly in the deployed instnace, bringing it into `master` now).
Test Plan: should be fine.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, speck
Differential Revision: https://we.phorge.it/D25048
Summary: Ref T13682. Allow users to manually attach files which are referenced (but not attached) via the UI.
Test Plan: Reference files via `{F...}`, then attached them via the UI workflow.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21837
Summary:
Ref T13603. Allow "VersionedDraft" to persist remarkup comment area metadata from stacked actions controls.
When files are dragged and dropped, record them as explicit uploads in comment metadata.
Test Plan: Dragged and dropped files into Remarkup stacked action text areas (e.g., in Maniphest), reloaded page, saw metadata persist across reloads.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21828
Summary: Ref T13603. This will support explicit handling of attached files.
Test Plan: Adjusted new input to have "text" input type, used it alongside additional upcoming changes, saw sensible metadata behavior.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21827
Summary:
Ref T9530. Ref T13658. The "Releeph" application was never useful outside of Facebook and any application providing release support would not resemble it much.
It has some product name literal strings, so now is as good a time as any to get rid of it.
This application never left prototype and I'm not aware of any install in the wild that uses it (or has ever used it).
I did not destroy the database itself. I'll issue upgrade guidance and destroy the database in some future release, just in case.
Test Plan: Grepped for "releeph", found no relevant/removable hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658, T9530
Differential Revision: https://secure.phabricator.com/D21792
Summary: This adds the Ablillity to translate the Content of the Date Picker. It is possible to display more than one Letter for the Day Captions.
Test Plan:
- Change the Language in the User Preferences, make sure it translates 'S|M|T|W|T|F|S' and Months or add the following to the translation.override config to get German Translation:
```
{
'S|M|T|W|T|F|S': 'So|Mo|Di|Mi|Do|Fr|Sa',
'January': 'Januar',
'February': 'Februar',
'March': 'März',
'May': 'Mai',
'June': 'Juni',
'July': 'Juli',
'October': 'Oktober',
'December': 'Dezember'
}
```
- Open Datepicker on a Datefield (Edit Event or Edit for Custom Field of Type date)
- The Day Headers and the Months should now be translated
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: CSharp, 0, speck, tobiaswiese
Tags: #calendar
Differential Revision: https://we.phorge.it/D25016
Summary: Ref T13072. Make large Conduit doc pages a bit more navigable. This prepares for updating "harbormaster.sendmessage" to support sending messages to builds.
Test Plan: Viewed various Conduit API documentation pages, clicked links.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21696
Summary:
Ref T13658. This just scrubs some of the simple references from the codebase.
Most of what's left is in documentation which won't be relevant for a fork and/or which I need to separately revise (or more-or-less delete) at some point anyway.
I removed the "install RHEL" and "install Ubuntu" scripts outright since I don't have any reasonable way to test them and don't plan to maintain them.
Test Plan: Grepped for "phacility", "epriestley"; ran unit tests.
Reviewers: cspeckmim
Reviewed By: cspeckmim
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21678
Summary:
The datepicker could step by the wrong number of months, due to the date rolling over to the next month when the number of days in the month is exceeded. For example, going forward from January 31 would jump to March 3, while going backward from July 31 would only go to July 1.
Push the date back to ensure that the datepicker stays in the correct month when switching.
Test Plan: Changed months starting from an assortment of dates.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: artms, Korvin
Differential Revision: https://secure.phabricator.com/D21673
Summary: Ref T13559. Substantially correct the client logic for "Save" and "Cancel" actions to handle unusual cases.
Test Plan:
Quoting behavior:
- Quoted a comment.
- Cancelled the quoted comment without modifying anything.
- Reloaded page.
- Before changes: quoted comment still exists.
- After changes: quoted comment is deleted.
- Looked at comment count in header, saw consistent behavior (before: weird behavior).
Empty suggestion behavior:
- Created a new comment on a suggestable file.
- Clicked "Suggest Edit" to enable suggestions.
- Without making any text or suggestion changes, clicked "Save".
- Before changes: comment saves, but is empty.
- After changes: comment deletes itself without undo.
General behavior:
- Created and saved an empty comment (deletes itself).
- Created and saved a nonempty comment (saves as draft).
- Created and saved an empty comment with an edit suggestion (saves).
- Created and saved an empty comment with a suggestion to entirely delete lines -- that is, no suggestion text (saves).
- Edited a comment, saved without changes (save).
- Edited a comment, save deleting all text (saves -- note that this is intentionally without undo, since this is a lot of steps to do by accident).
- Cancel editing an unchanged comment (cancels without undo).
- Cancel editing a changed comment (cancels with undo).
- Undo'd, got text back.
- Cancel new comment with no text (deletes without undo).
- Cancel new comment with text (deletes with undo).
- Undo'd, got text back.
- Saved a quoted comment with no changes (saves -- note that this is intentionally not a "delete", since just quoting someone seems fine if you click "Save" -- maybe you want to come back to it later).
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21654
Summary:
Ref T13559. Various client decisions depend on the "initial" or "committed" states of inline comments. Previously, these were informally constructed from "mostly similar" available values, or glossed over in some cases.
On the server, save the initial state when creating a comment. Save the committed state when applying a "save" operation. Send all three states to the client.
On the client, load and track all three states explicitly.
Test Plan: Created inlines, etc. See followups.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21653
Summary:
Ref T13559. Currently, the default text for inline comment side-loads in a bizarre way. Instead, when a user creates an inline comment, load the inline context and set it as part of the initial content state.
This allows the side channel (and the code that puts the text in place at the last second on the client) to be removed.
Test Plan: Created inlines, clicked "Suggest Edit". See followup changes.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21652
Summary:
Ref T13559. To allow the client to make correct decisions about what buttons mean, track an explicit "Committed" content state.
This is the last version of the comment that has been saved on the server, and does not exist if the comment has never been saved.
Test Plan: Created comments, etc. See followups.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21651
Summary:
Ref T13559. When the user clicks the "Cancel" button, we sometimes take it to mean "delete" (when the comment is empty).
Both the client and server make a decision about this, and they may not agree, which causes the client to fall out of sync.
Make the client responsible for deciding whether it wants to interpret a click on the "Cancel" button as a "revert" or a "delete".
Test Plan: Cancelled empty and nonempty comments, etc. See followup changes.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21649
Summary:
Ref T13559. When you click "Save" on an inline comment and it's empty, we may actually delete the comment.
Currently, the client and server both make decisions about whether the comment should be deleted. These decisions may not agree, causing the client state to fall out of sync.
Make the client authoritative about whether it wants to handle the user clicking the "Save" button as an intent to save or an intent to delete.
Test Plan: Saved empty and nonempty inlines. See followup changes.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21648
Summary:
Ref T13559. Instead of directly reading form state, make all callers use the "active" state instead. The state reads the form.
No functional changes, just clarifying responsiblites.
Test Plan: Created inlines, etc. See followup changes.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21647
Summary:
Ref T13559. Rather than reading from the document, make client inlines actively track their current "active" state.
The "active" state is what the user currently sees in the client UI.
Test Plan: Created inlines, etc. See followups.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21646
Summary:
Ref T13559. In an effort to ultimately fix the "quote + cancel" bug, begin formalizing content states on the client.
This creates a "ContentState" client object and moves the authoritative storage for the "hasSuggestion" property to it.
Test Plan: Created inlines, etc. See followups.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21645
Summary:
Ref T13559. D21261 added caching here, but the logic in rebuilding inlines wasn't quite correct, and could lead to us double-appending.
Instead, when rebuilding, unconditionally discard the old list.
Test Plan:
- Added inline comments to a file in Differential.
- Marked some done.
- Scrolled so the inline comment header was visible, saw "X / Y Comments" button in header.
- Clicked "Show 20 more lines" on the changeset with inlines (or toggle "View Unified" / "View Side-by-Side", or other interactions likely work too).
- Before: saw "X / Y" change improperly (because inlines in that file were double-counted).
- After: saw stable count.
- Grepped for "differential-inline-comment-refresh", got no hits, concluded this event has no listeners.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21642
Summary:
Ref T13644. Ref T13638.
- Double-encode the symbol that is used as a path component, similar to Diffusion.
- Fix an outdated reference to ".path", which provided context for symbol lookup.
- Prevent command-clicking headers from looking up the path as a symbol.
Test Plan:
- Command-clicked headers, no longer got a symbol.
- Command-clicked stuff with "/", saw it double-encoded and decoded properly.
- Command-clicked normal symbols, saw "path" populate correctly.
Maniphest Tasks: T13644, T13638
Differential Revision: https://secure.phabricator.com/D21641
