WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
178795
Web Inspector: Styles Redesign: typing colon in property name should advance to value field
https://bugs.webkit.org/show_bug.cgi?id=178795
Summary
Web Inspector: Styles Redesign: typing colon in property name should advance ...
Blaze Burg
Reported
2017-10-25 09:02:24 PDT
Steps to Reproduce: 1. Open style rules for an element 2. Add a new property name, add trailing : 3. Tab to the next field => property has two colons, should just have one and advance I don't think it's valid to have a colon in a property name, especially at the end, so the editor should be nice and ignore the keystroke while advancing to the next field. If you are used to typing out a complete property: value; string, then it will just do the right thing. This is similar to
https://bugs.webkit.org/show_bug.cgi?id=178498
, but for properties.
Attachments
Patch
(7.09 KB, patch)
2017-11-14 18:05 PST
,
Nikita Vasilyev
hi
: review+
Details
Formatted Diff
Diff
Patch
(7.33 KB, patch)
2017-11-15 14:04 PST
,
Nikita Vasilyev
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2017-10-25 09:02:38 PDT
<
rdar://problem/35174674
>
Nikita Vasilyev
Comment 2
2017-11-14 18:05:13 PST
Created
attachment 326953
[details]
Patch Typing colon ANYWHERE in the property name advances to value field. This matches Chrome and Firefox. I could remove text on the right side of colon and prepend it to property value. I wasn't sure if it's useful, so I didn't do it in this patch.
Devin Rousso
Comment 3
2017-11-14 19:55:42 PST
Comment on
attachment 326953
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=326953&action=review
r=me
> Source/WebInspectorUI/ChangeLog:29 > + (WI.SpreadsheetTextField):
NIT: this should be at the beginning of the WI.SpreadsheetTextField list. This happens to me every once in a while for some reason ¯\_(ツ)_/¯
> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:177 > + this._nameElement.addEventListener("beforeinput", (event) => {
Since you are adding the event listener to the element itself, I'd reorder these. this._nameElement.tabIndex = 0; this._nameElement.addEventListener("beforeinput", this._handleNameBeforeinput.bind(this)); this._nameTextField = new WI.SpreadsheetTextField(this, this._nameElement, this._nameCompletionDataProvider.bind(this)); I'd also suggest moving the callback to be a member function. I typically try to only use inline event listeners for extremely small (effectively one line) callbacks, or contextmenu stuff.
Nikita Vasilyev
Comment 4
2017-11-15 14:04:02 PST
Created
attachment 327023
[details]
Patch
WebKit Commit Bot
Comment 5
2017-11-15 14:57:11 PST
The commit-queue encountered the following flaky tests while processing
attachment 327023
[details]
: The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 6
2017-11-15 16:31:03 PST
Comment on
attachment 327023
[details]
Patch Clearing flags on attachment: 327023 Committed
r224906
: <
https://trac.webkit.org/changeset/224906
>
WebKit Commit Bot
Comment 7
2017-11-15 16:31:05 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug