Paths

Table of Contentst

-
webroot/rsrc/externals/javelin/lib/
-
rsrc/
-
externals/
-
javelin/
-
lib/
1/1
Workflow.js

Fix regression in new confirmation Dialog
ClosedPublic
Actions

Authored by valerio.bozzolan on Mar 13 2023, 20:40.

Details

Reviewers

bekay
Ekubischta
avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15034: Show confirmation dialog when closing a modal if form contents have been changed

Commits

rP3487ee444a88: Fix regression in new confirmation Dialog

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

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

valerio.bozzolan created this revision.Mar 13 2023, 20:40

Owners added a reviewer: O1: Blessed Committers.Mar 13 2023, 20:40

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptMar 13 2023, 20:40

valerio.bozzolan requested review of this revision.Mar 13 2023, 20:40

Harbormaster completed remote builds in B133: Diff 340.Mar 13 2023, 20:41

valerio.bozzolan mentioned this in D25015: Show confirmation dialog when closing a modal if form contents have been changed.Mar 13 2023, 20:45

valerio.bozzolan mentioned this in T15034: Show confirmation dialog when closing a modal if form contents have been changed.Mar 13 2023, 20:48

valerio.bozzolan mentioned this in 2023 Week 2.

adopt window.confirm instead of just confirm to make linter happy

https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm

Harbormaster completed remote builds in B134: Diff 341.Mar 14 2023, 07:17

valerio.bozzolan mentioned this in Z1: Phorge.Mar 14 2023, 07:19

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 14 2023, 13:38

avivey accepted this revision.Mar 14 2023, 18:32

This revision is now accepted and ready to land.Mar 14 2023, 18:32

Closed by commit rP3487ee444a88: Fix regression in new confirmation Dialog. · Explain WhyMar 14 2023, 19:25

This revision was automatically updated to reflect the committed changes.

valerio.bozzolan added a commit: rP3487ee444a88: Fix regression in new confirmation Dialog.

Dylsss mentioned this in D25052: Add and use new RemarkupMetadata class.Apr 24 2023, 00:05

Revision Contents
Changeset List

Path

Size

Packages

webroot/

rsrc/

externals/

javelin/

lib/

Workflow.js

41 lines

O1: Blessed Committers

Diff 342

View Options

webroot/rsrc/externals/javelin/lib/Workflow.js

Show All 37 Lines	public function setIcon($icon) {
$this->icon = $icon;		$this->icon = $icon;
return $this;		return $this;
}		}

public function getIcon() {		public function getIcon() {
return $this->icon;		return $this->icon;
}		}

		/**
		* Set the text of the tag
		*
		* This is usually the 'name' key of your subtype map.
		* Sometime this is an uppercase text like 'BUG' for a 'bug' subtype name.
		*
		* @param string\|null $text
		* @return self
		*/
public function setTagText($text) {		public function setTagText($text) {
$this->tagText = $text;		$this->tagText = $text;
return $this;		return $this;
}		}

		/**
		* Get the text of the tag
		*
		* @see PhabricatorEditEngineSubtype::setTagText()
		*
		* @return string\|null
		*/
public function getTagText() {		public function getTagText() {
return $this->tagText;		return $this->tagText;
}		}

public function setColor($color) {		public function setColor($color) {
$this->color = $color;		$this->color = $color;
return $this;		return $this;
}		}
Show All 25 Lines	public function setMutations($mutations) {
return $this;		return $this;
}		}

public function getMutations() {		public function getMutations() {
return $this->mutations;		return $this->mutations;
}		}

public function hasTagView() {		public function hasTagView() {
return (bool)strlen($this->getTagText());		return phutil_nonempty_string($this->getTagText());
		valerio.bozzolanUnsubmitted Done Inline Actions ✅ The only usage of `setTagText()` seems to be in `PhabricatorEditEngineSubtype` and the input domain is just a `string` coming from `phutil_utf8_strtoupper()` (like `"BUG"`) or `null` (default). The `phutil_nonempty_string()` will report alien values and this is OK. Also, the method was returning a bool, and it still returns a bool. valerio.bozzolan: ✅ The only usage of `setTagText()` seems to be in `PhabricatorEditEngineSubtype` and the input…
}		}

public function newTagView() {		public function newTagView() {
$view = id(new PHUITagView())		$view = id(new PHUITagView())
->setType(PHUITagView::TYPE_OUTLINE)		->setType(PHUITagView::TYPE_OUTLINE)
->setName($this->getTagText());		->setName($this->getTagText());

$color = $this->getColor();		$color = $this->getColor();
▲ Show 20 Lines • Show All 217 Lines • Show Last 20 Lines

Fix regression in new confirmation DialogClosedPublicActions