Skip to content

Commit 2b69711

Browse files
aboseabose
committed
chore: ux improvement fir fix problems quick views and gutter
1 parent 676ff06 commit 2b69711

4 files changed

Lines changed: 56 additions & 17 deletions

File tree

‎src/language/CodeInspection.js‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,20 @@ define(function (require, exports, module) {
7676
META: "meta"
7777
};
7878

79-
function _getIconClassForType(type) {
79+
function _getIconClassForType(type, isFixable) {
8080
switch (type) {
81-
case Type.ERROR: return "line-icon-problem_type_error fa-solid fa-times-circle";
82-
case Type.WARNING: return "line-icon-problem_type_warning fa-solid fa-exclamation-triangle";
83-
case Type.META: return "line-icon-problem_type_info fa-solid fa-info-circle";
84-
default: return "line-icon-problem_type_info fa-solid fa-info-circle";
81+
case Type.ERROR: return isFixable ?
82+
"line-icon-problem_type_error fa-solid fa-wrench":
83+
"line-icon-problem_type_error fa-solid fa-times-circle";
84+
case Type.WARNING: return isFixable ?
85+
"line-icon-problem_type_warning fa-solid fa-wrench":
86+
"line-icon-problem_type_warning fa-solid fa-exclamation-triangle";
87+
case Type.META: return isFixable ?
88+
"line-icon-problem_type_info fa-solid fa-wrench":
89+
"line-icon-problem_type_info fa-solid fa-info-circle";
90+
default: return isFixable ?
91+
"line-icon-problem_type_info fa-solid fa-wrench":
92+
"line-icon-problem_type_info fa-solid fa-info-circle";
8593
}
8694
}
8795

@@ -434,17 +442,18 @@ define(function (require, exports, module) {
434442
* @param ch - the character position of the error
435443
* @param type - The type of the marker. This is a string that can be one of the error types
436444
* @param message - The message that will be displayed when you hover over the marker.
445+
* @param isFixable - true if we need to use the fix icon
437446
* @returns A DOM element.
438447
*/
439-
function _createMarkerElement(editor, line, ch, type, message) {
448+
function _createMarkerElement(editor, line, ch, type, message, isFixable) {
440449
let $marker = $('<div><span>')
441450
.attr('title', message)
442451
.addClass(CODE_INSPECTION_GUTTER);
443452
$marker.click(function (){
444453
editor.setCursorPos(line, ch);
445454
});
446455
$marker.find('span')
447-
.addClass(_getIconClassForType(type))
456+
.addClass(_getIconClassForType(type, isFixable))
448457
.addClass("brackets-inspection-gutter-marker")
449458
.html('&nbsp;');
450459
return $marker[0];
@@ -477,7 +486,11 @@ define(function (require, exports, module) {
477486
for(let lineno of Object.keys(gutterErrorMessages)){
478487
// We mark the line with the Highest priority icon. (Eg. error icon if same line has warnings and info)
479488
let highestPriorityMarkTypeSeen = Type.META;
489+
let fixableMarkFound = false;
480490
let gutterMessage = gutterErrorMessages[lineno].reduce((prev, current)=>{
491+
if(current.fixable || prev.fixable){
492+
fixableMarkFound = true;
493+
}
481494
if(_getMarkTypePriority(current.type) > _getMarkTypePriority(highestPriorityMarkTypeSeen)){
482495
highestPriorityMarkTypeSeen = current.type;
483496
}
@@ -486,7 +499,7 @@ define(function (require, exports, module) {
486499
let line = gutterErrorMessages[lineno][0].line,
487500
ch = gutterErrorMessages[lineno][0].ch,
488501
message = gutterMessage.message;
489-
let marker = _createMarkerElement(editor, line, ch, highestPriorityMarkTypeSeen, message);
502+
let marker = _createMarkerElement(editor, line, ch, highestPriorityMarkTypeSeen, message, fixableMarkFound);
490503
editor.setGutterMarker(line, CODE_INSPECTION_GUTTER, marker);
491504
}
492505
_populateDummyGutterElements(editor, 0, editor.getLastVisibleLine());
@@ -531,15 +544,18 @@ define(function (require, exports, module) {
531544
if(documentFixes.get(fixID)){
532545
$problemView = $(`<div>
533546
<button class="btn btn-mini primary fix-problem-btn" style="margin-right: 5px;">Fix</button>
534-
${mark.message}<br/>
547+
<a style="cursor:pointer;">${mark.message}</a>
548+
<br/>
535549
</div>`);
536550
$problemView.find(".fix-problem-btn").click(()=>{
537551
scrollToProblem(pos.line);
538552
_fixProblem(fixID);
539553
});
540554
$hoverMessage.append($problemView);
541555
} else {
542-
$problemView = $(`<div>${mark.message}<br/></div>`);
556+
$problemView = $(`<div>
557+
<a style="cursor: pointer">${mark.message}</a>
558+
<br/></div>`);
543559
$hoverMessage.append($problemView);
544560
}
545561
// eslint-disable-next-line no-loop-func
@@ -615,9 +631,7 @@ define(function (require, exports, module) {
615631
for (let error of errors) {
616632
let line = error.pos.line || 0;
617633
let ch = error.pos.ch || 0;
618-
let gutterMessage = gutterErrorMessages[line] || [];
619-
gutterMessage.push({message: error.message, type: error.type, line, ch});
620-
gutterErrorMessages[line] = gutterMessage;
634+
let fixable = false;
621635
// add squiggly lines
622636
if (_shouldMarkTokenAtPosition(editor, error)) {
623637
let mark;
@@ -626,6 +640,7 @@ define(function (require, exports, module) {
626640
if(fixID) {
627641
markOptions.metadata = fixID;
628642
error.fix.id = fixID;
643+
fixable = true;
629644
}
630645
if(error.endPos && !editor.isSamePosition(error.pos, error.endPos)) {
631646
mark = editor.markText(CODE_MARK_TYPE_INSPECTOR, error.pos, error.endPos, markOptions);
@@ -635,6 +650,9 @@ define(function (require, exports, module) {
635650
mark.type = error.type;
636651
mark.message = error.message;
637652
}
653+
let gutterMessage = gutterErrorMessages[line] || [];
654+
gutterMessage.push({message: error.message, type: error.type, fixable, line, ch});
655+
gutterErrorMessages[line] = gutterMessage;
638656
}
639657
}
640658
_updateGutterMarks(editor, gutterErrorMessages);

‎src/nls/root/strings.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ define({
410410

411411
// CodeInspection: errors/warnings
412412
"ERRORS_NO_FILE": "No File Open",
413+
"ERRORS_CLICK_TO_VIEW_PROBLEM": "Click to view problem",
413414
"ERRORS_PANEL_TITLE_MULTIPLE": "{0} Problems - {1}",
414415
"ERRORS_PANEL_TITLE_MULTIPLE_FIXABLE": "{0} Problems, {1} Fixable - {2}",
415416
"SINGLE_ERROR": "1 {0} Problem - {1}",

‎test/UnitTestReporter.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,20 @@ define(function (require, exports, module) {
351351
return result;
352352
}
353353
};
354+
}, toIncludeText: function() {
355+
return {
356+
compare: function(actual, expected) {
357+
const pass = actual.includes(expected);
358+
const message = pass ?
359+
`Expected "${actual}" to include text "${expected}"` :
360+
`Expected "${actual}" to include text "${expected}", but it did not`;
361+
362+
return {
363+
pass: pass,
364+
message: message
365+
};
366+
}
367+
};
354368
}
355369
});
356370
}

‎test/spec/Extn-CSSCodeHints-integ-test.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,16 @@ define(function (require, exports, module) {
8282
}
8383
}
8484

85-
async function checkCSSWarningAtPos(expectedWarning, line, ch) {
85+
async function checkCSSWarningAtPos(expectedWarning, line, ch, useIncludes) {
8686
await awaitsFor(async ()=>{
8787
return await getPopoverAtPos(line, ch);
8888
}, "popover to be present");
8989
let popoverInfo = await getPopoverAtPos(line, ch);
90-
expect(popoverInfo.content.find(".code-inspection-item").text().trim()).toBe(expectedWarning);
90+
if(useIncludes){
91+
expect(popoverInfo.content.find(".code-inspection-item").text().trim()).toIncludeText(expectedWarning);
92+
} else {
93+
expect(popoverInfo.content.find(".code-inspection-item").text().trim()).toBe(expectedWarning);
94+
}
9195
}
9296

9397
function runTests(testTyle) {
@@ -146,8 +150,10 @@ define(function (require, exports, module) {
146150
});
147151

148152
it(`Should ${testTyle} show unknown vendor prefix warning`, async function () {
149-
await checkCSSWarningAtPos("Unknown vendor specific property. (unknownVendorSpecificProperties)Also define the standard property 'border-radius' for compatibility (vendorPrefix)",
150-
43, 8);
153+
await checkCSSWarningAtPos("Unknown vendor specific property. (unknownVendorSpecificProperties)",
154+
43, 8, true);
155+
await checkCSSWarningAtPos("Also define the standard property 'border-radius' for compatibility (vendorPrefix)",
156+
43, 8, true);
151157
});
152158
}
153159

0 commit comments

Comments
 (0)