Add a new RemarkupMetadata class and use with upload button as well as drag and drop pathways.
Details
Details
- Reviewers
avivey Matthew - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15106: "Upload file" in remarkup text fields should attach by default
Drag and drop file, upload file with button. Check that both files are attached with "attachedFilePHIDs" values.
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- master
- Lint
Lint Warnings Severity Location Code Message Warning webroot/rsrc/js/core/RemarkupMetadata.js:1 JAVELIN5 `javelinsymbols` Not In Path - Unit
Test Failures - Build Status
Buildable 85 Build 85: arc lint + arc unit
| Time | Test | |
|---|---|---|
| 703 ms | PhabricatorCelerityTestCase::testCelerityMaps Assertion failed, expected 'true' (at PhabricatorCelerityTestCase.php:32): When this test fails, it means the Celerity resource map is out of date. Run `bin/celerity map` to rebuild it.
ACTUAL VALUE
| |
| 50 ms | PhabricatorAphrontViewTestCase::testHasChildren 6 assertions passed. | |
| 772 ms | PhabricatorConduitTestCase::testConduitMethods 1 assertion passed. | |
| 425 ms | PhabricatorInfrastructureTestCase::testApplicationsInstalled 1 assertion passed. | |
| 421 ms | PhabricatorInfrastructureTestCase::testRejectMySQLNonUTF8Queries 1 assertion passed. | |
| View Full Test Results (1 Failed · 11 Passed) |
Event Timeline
Comment Actions
Differentiate between metadata of different text areas using node ID. e.g., new comment text areas and text areas in the edit comment dialog.
Comment Actions
Bear with me here, because Javeline (like all js frameworks) is confusing to me.
I'd expect some PHP code to actually attach the files from attachedFilePHIDs to the relevant object, but I don't see it in this diff. Is that already happening somehow?
| webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js | ||
|---|---|---|
| 260–264 | is metadata gets automatically inserted into something? I don't see it being manually saved. | |
Comment Actions
| webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js | ||
|---|---|---|
| 260–264 | It gets written to the HTML element in the setMetadata method. | |
Comment Actions
Please rebase and resubmit, there is a merge conflict.
matthew@Tower phorge % arc patch D25052
Created and checked out branch arcpatch-D25052.
Checking patch webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js...
Checking patch webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js...
Checking patch webroot/rsrc/js/core/RemarkupMetadata.js...
Checking patch src/view/form/control/PhabricatorRemarkupControl.php...
Checking patch resources/celerity/map.php...
Applied patch webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js cleanly.
Applied patch webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js cleanly.
Applied patch webroot/rsrc/js/core/RemarkupMetadata.js cleanly.
Applied patch src/view/form/control/PhabricatorRemarkupControl.php cleanly.
Applied patch resources/celerity/map.php cleanly.
COMMITTED Successfully committed patch.
matthew@Tower phorge % arc land
STRATEGY Merging with "squash" strategy, the default strategy.
SOURCE Landing the current branch, "arcpatch-D25052".
ONTO REMOTE Landing onto remote "origin", the default remote under Git.
ONTO TARGET Landing onto target "master", the default target under Git.
INTO REMOTE Will merge into remote "origin" by default, because this is the remote the change is landing onto.
INTO TARGET Will merge into target "master" by default, because this is the "onto" target.
FETCH Fetching "master" from remote "origin"...
$ git fetch --no-tags --quiet -- origin master
INTO COMMIT Preparing merge into "master" from remote "origin", at commit "dc558b5538cd".
LANDING These changes will land:
* D25052 Add and use new RemarkupMetadata class
27935e60780f Add and use new RemarkupMetadata class
>>> Land these changes? [y/N/?] y
<!> NOT REVISION AUTHOR
You are landing revisions which you ("@Matthew") are not the author of:
* D25052 Add and use new RemarkupMetadata class
Author: @Dylsss
? Use "Commandeer" in the web interface to become the author of a
revision.
>>> Land revisions you are not the author of? [y/N/?] y
<!> BUILD FAILURES
1 revision(s) have build failures:
* D25052 Add and use new RemarkupMetadata class
* B87 Buildable "B87"
:// https://we.phorge.it/B87
* Build 87 arc lint + arc unit
>>> Land 1 revision(s) anyway, despite failed builds? [y/N/?] y
MERGING 27935e60780f Add and use new RemarkupMetadata class
MERGE Attempting to rebase changes.
MERGE Attempting to reduce and rebase changes.
<!> MERGE CONFLICT
Local commit "27935e60780f" ("arcpatch-D25052") does not merge cleanly into
"dc558b5538cd". Rebase or merge local changes so they can merge cleanly.
LOAD STATE Restoring local state (to ref "arcpatch-D25052" at commit "27935e60780f").
USAGE EXCEPTION Encountered a merge conflict.Comment Actions
For some historical context, this current patch comes from another one from the Phabricator of Wikimedia Foundation:
| webroot/rsrc/js/core/RemarkupMetadata.js | ||
|---|---|---|
| 13–18 | Thanks. In short, this is the proposed change. Taking this: if (JX.RemarkupMetadata._metadataValue == null) {
JX.RemarkupMetadata._metadataValue = [];
JX.RemarkupMetadata._metadataValue[metadataID] = metadataValue;
} else if (!JX.RemarkupMetadata._metadataValue.hasOwnProperty(metadataID)) {
JX.RemarkupMetadata._metadataValue[metadataID] = metadataValue;
}And simplifying it to this: if (JX.RemarkupMetadata._metadataValue == null) {
JX.RemarkupMetadata._metadataValue = {};
}
if (!JX.RemarkupMetadata._metadataValue.hasOwnProperty(metadataID)) {
JX.RemarkupMetadata._metadataValue[metadataID] = metadataValue;
}Reasons:
| |
This comment was removed by valerio.bozzolan.