ocket8888 commented on code in PR #7532:
URL: https://github.com/apache/trafficcontrol/pull/7532#discussion_r1228649316


##########
experimental/traffic-portal/src/app/api/profile.service.spec.ts:
##########
@@ -116,6 +136,27 @@ describe("ProfileService", () => {
                await expectAsync(responseP).toBeResolvedTo(profile);
        });
 
+       it("sends request for Export object by Profile ID", async () => {
+               const profileId = 1;
+               const id = typeof(profileId) === "number" ? profile.id : 
profileId;

Review Comment:
   The type of `profileId` is `const 1` so it will always be a `number`, so the 
branch using `profileId` will never be taken here. I'm not sure what you're 
trying to do, or I'd have something more helpful to say.



##########
experimental/traffic-portal/src/app/shared/import-json-txt/import-json-txt.component.html:
##########
@@ -0,0 +1,38 @@
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<h2 mat-dialog-title>{{data.title}}</h2>
+<mat-dialog-content id="import-content" [ngClass]="{'active':dragOn}">
+       <input type='file' id="imageUpload" aria-label="Select a file" 
accept="application/json,text/plain" #fileInput (change)="uploadFile($event)"/>
+       <button type="button" class="dropzone" (click)="fileInput.click()">
+               <div class="text-wrapper">
+                       <div class="label">

Review Comment:
   [a button may only contain phrasing 
content](https://html.spec.whatwg.org/#the-button-element). I think you 
probably want to make this instead a `label` of the `input` and put both within 
one or more flow content elements (e.g. `div`) that listen for drag events.
   
   Drag events should set the value of the input, by the way, since otherwise 
it could appear to users that dragging a file into the space doesn't change the 
input, which is confusing because then which is uploaded?



##########
experimental/traffic-portal/src/app/api/testing/profile.service.ts:
##########
@@ -215,11 +228,39 @@ export class ProfileService {
        public async deleteProfile(id: number | ResponseProfile): 
Promise<ResponseProfile> {
                const index = this.profiles.findIndex(t => t.id === id);
                if (index === -1) {
-                       throw new Error(`no such Type: ${id}`);
+                       throw new Error(`no such profile: ${id}`);
                }
                return this.profiles.splice(index, 1)[0];
        }
 
+       /**
+        * Export Profile object from the API.
+        *
+        * @param id Specify unique identifier (number) of a specific Profile 
to retrieve the export object.
+        * @returns The requested Profile as attachment.
+        */
+       public async exportProfile(id: number | ResponseProfile): 
Promise<ProfileExport> {
+               const index = this.profiles.findIndex(t => t.id === id);
+               if (index === -1) {
+                       throw new Error(`no such Profile: ${id}`);
+               }
+               return this.profileExport;
+       }

Review Comment:
   this method will always throw an error when passed a `ResponseProfile`, 
because the types `number` and `ResponseProfile` will never compare as equal 
using `===`.



##########
experimental/traffic-portal/src/app/shared/import-json-txt/import-json-txt.component.html:
##########
@@ -0,0 +1,38 @@
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<h2 mat-dialog-title>{{data.title}}</h2>
+<mat-dialog-content id="import-content" [ngClass]="{'active':dragOn}">
+       <input type='file' id="imageUpload" aria-label="Select a file" 
accept="application/json,text/plain" #fileInput (change)="uploadFile($event)"/>
+       <button type="button" class="dropzone" (click)="fileInput.click()">
+               <div class="text-wrapper">
+                       <div class="label">
+                               Drop your file here!<br>
+                               <small class="hint">{{mimeAlertMsg}}</small>
+                       </div>
+               </div>
+       </button>
+
+       <ul id="file-name">
+               <li *ngIf="fileData">{{fileData}}</li>
+       </ul>
+
+       <div id="json-txt" *ngIf="inputTxt">
+               <pre>{{inputTxt | json}}</pre>
+       </div>
+</mat-dialog-content>
+<mat-dialog-actions align="end">
+       <button mat-raised-button type="button" mat-dialog-close 
color="warn">Cancel</button>
+       <button mat-raised-button type="button" (click)="submit()" 
[disabled]="!fileData">Submit</button>

Review Comment:
   without either a mat-dialog-close or submit action, this button seems to 
accessibility tools to be totally unrelated to any other control on the page, 
including the file inputs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to