Page MenuHomePhorge
Differential D25037

Hide the "hidden" fields on custom form previews
ClosedPublic
Actions

Authored by 20after4 on Apr 24 2022, 18:19.

Details

Reviewers
Cigaryno
avivey
valerio.bozzolan
Matthew
Group Reviewers
Blessed Committers
O1: Blessed Committers
Maniphest Tasks
T15217: Make 'hidden' fields actually hidden on the Phorge form preview view
Commits
rP821df3364ec4: Hide the "hidden" fields on custom form previews
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.

Diff Detail

Repository
rP Phorge
Branch
arcpatch-D25037_1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 169
Build 169: arc lint + arc unit

Event Timeline

20after4 created this revision.Apr 24 2022, 18:19
Owners added a reviewer: O1: Blessed Committers.Apr 24 2022, 18:19
Herald added subscribers: Matthew, valerio.bozzolan, tobiaswiese, speck. · View Herald TranscriptApr 24 2022, 18:19
20after4 requested review of this revision.Apr 24 2022, 18:19
Harbormaster completed remote builds in B66: Diff 92.Apr 24 2022, 18:19
20after4 mentioned this in T15081: Figure out if there are patches from Wikimedia's fork that are desirable to upstream in Phorge.Apr 24 2022, 18:39
20after4 added a task: T15081: Figure out if there are patches from Wikimedia's fork that are desirable to upstream in Phorge.
Cigaryno subscribed.Sep 13 2022, 14:21
Cigaryno accepted this revision.Oct 6 2022, 19:08

LGTM, we may also upstream more Wikimedia patches, per T15081: Figure out if there are patches from Wikimedia's fork that are desirable to upstream in Phorge.

avivey accepted this revision.Oct 28 2022, 10:28
avivey subscribed.

I'm good with this, but we have a policy of "no landed diffs w/o tickets", so please create one before landing.

This revision is now accepted and ready to land.Oct 28 2022, 10:28
Cigaryno added a comment.Oct 30 2022, 09:31
In D25037#1857, @avivey wrote:

I'm good with this, but we have a policy of "no landed diffs w/o tickets", so please create one before landing.

This revision is part of T15081.

Matthew requested changes to this revision.Dec 8 2022, 22:34

This does not land cleanly, please merge from master and re-submit.

matthew@Tower phorge % arc land
[2022-12-08 23:33:09] EXCEPTION: (PhutilBootloaderException) The phutil library 'phorge' has not been loaded! at [<arcanist>/src/init/lib/PhutilBootloader.php:199]
PHP Fatal error:  Uncaught exception 'PhutilBootloaderException' with message 'The phutil library 'phorge' has not been loaded!' in /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/PhutilBootloader.php:199
Stack trace:
#0 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/moduleutils.php(5): PhutilBootloader->getLibraryRoot('phabricator')
#1 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(482): phutil_get_library_root('phabricator')
#2 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(317): PhutilErrorHandler::getLibraryVersions()
#3 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(300): PhutilErrorHandler::formatStacktrace(Array)
#4 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(417): PhutilErrorHandler::outputStacktrace(Array)
#5 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(282): PhutilErrorHandler::dispatchErrorMessage('exception', Object( in /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/PhutilBootloader.php on line 199

Fatal error: Uncaught exception 'PhutilBootloaderException' with message 'The phutil library 'phorge' has not been loaded!' in /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/PhutilBootloader.php:199
Stack trace:
#0 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/moduleutils.php(5): PhutilBootloader->getLibraryRoot('phabricator')
#1 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(482): phutil_get_library_root('phabricator')
#2 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(317): PhutilErrorHandler::getLibraryVersions()
#3 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(300): PhutilErrorHandler::formatStacktrace(Array)
#4 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(417): PhutilErrorHandler::outputStacktrace(Array)
#5 /Users/matthew/PhpstormProjects/Phorge/arcanist/src/error/PhutilErrorHandler.php(282): PhutilErrorHandler::dispatchErrorMessage('exception', Object( in /Users/matthew/PhpstormProjects/Phorge/arcanist/src/init/lib/PhutilBootloader.php on line 199
This revision now requires changes to proceed.Dec 8 2022, 22:34
valerio.bozzolan updated this revision to Diff 381.Mar 24 2023, 09:09

rebase from master

Harbormaster completed remote builds in B169: Diff 381.Mar 24 2023, 09:09
valerio.bozzolan accepted this revision as: valerio.bozzolan.Mar 24 2023, 23:00
valerio.bozzolan accepted this revision.Mar 24 2023, 23:05

(I accept also as O1 since somebody from O1 tried to land)

valerio.bozzolan added a comment.Mar 24 2023, 23:08

Hi @Matthew please undo your -1 so the author can land

(My previous notification is nonsense - sorry)

Matthew resigned from this revision.Mar 24 2023, 23:09

Sure, done.

This revision is now accepted and ready to land.Mar 24 2023, 23:09
valerio.bozzolan added a comment.Mar 25 2023, 14:41

Green light @20after4 - you can land - thank you

valerio.bozzolan added a comment.Mar 27 2023, 06:41

(In 3 days I will assume a "Yeah my friends please Land this for me")

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 27 2023, 16:24
valerio.bozzolan edited the test plan for this revision. (Show Details)
valerio.bozzolan added a comment.Mar 30 2023, 10:03
In D25037#3002, @valerio.bozzolan wrote:

(In 3 days I will assume a "Yeah my friends please Land this for me")

  • As mentioned here ↑
  • since I trust the proposal user
  • since this is only a change in CSS
  • since this does not break anything
  • since I tried this in my local installation with arc patch D25037 and I like its result
  • since in CC there are 5 people

I will help the author to land this.

sgtm

valerio.bozzolan updated this revision to Diff 428.Mar 30 2023, 10:08

I've run this:

./bin/celerity map
Harbormaster completed remote builds in B204: Diff 428.Mar 30 2023, 10:08
valerio.bozzolan updated this revision to Diff 429.Mar 30 2023, 10:11
  • update Celerify (again)
Harbormaster completed remote builds in B205: Diff 429.Mar 30 2023, 10:11
Closed by commit rP821df3364ec4: Hide the "hidden" fields on custom form previews. · Explain WhyMar 30 2023, 10:11
This revision was automatically updated to reflect the committed changes.
valerio.bozzolan added a commit: rP821df3364ec4: Hide the "hidden" fields on custom form previews.
valerio.bozzolan mentioned this in 2023 Week 2.Mar 30 2023, 10:19
valerio.bozzolan edited tasks, added T15217: Make 'hidden' fields actually hidden on the Phorge form preview view; removed T15081: Figure out if there are patches from Wikimedia's fork that are desirable to upstream in Phorge.
valerio.bozzolan mentioned this in rP821df3364ec4: Hide the "hidden" fields on custom form previews.Mar 30 2023, 14:14
avivey mentioned this in 2023 Week 17.Apr 25 2023, 06:54

Revision Contents
Changeset List

PathSizePackages
webroot/
rsrc/
css/
phui/
2 linesO1: Blessed Committers

Diff 381

View Options

webroot/rsrc/css/phui/phui-form-view.css

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0