search

input-output-hk / dapps-certification-web

Certification as a service for the Cardano dApps platform
Apache License 2.0
5 stars 0 forks source link

[PLT-7344] Revamp - Certification/Testing Page #13

Closed amnambiar closed 1 year ago

amnambiar commented 1 year ago

https://input-output.atlassian.net/browse/PLT-7344 https://input-output.atlassian.net/browse/PLT-7052

BE: PR #90 origin/feat/l1-certificate (also contains profile field modifications)

@coderabbitai: ignore

Summary by CodeRabbit

Release Notes:

coderabbitai[bot] commented 1 year ago

Walkthrough

This pull request introduces significant changes across the application, focusing on performance optimization, user experience enhancement, and code maintainability. It includes updates to HTML, TypeScript, React components, CSS stylesheets, and test cases. The changes range from simplifying external dependencies in HTML, improving error handling in API requests, restructuring routing configuration, enhancing component styling, to updating state management in React components.

Changes

File(s) Summary
public/index.html Optimized performance by reducing external requests and removing unnecessary dependencies.
src/api/onRequest.ts Improved error handling for unauthorized requests.
src/app/App.tsx Restructured route hierarchy and updated suspense fallback component.
src/components/Button/... Enhanced button styling and customization options.
src/components/CreateCertificate/CreateCertificate.tsx Updated state management and function calls.
src/components/LogsView/... Renamed components, updated props, and improved scroll behavior. Test cases are currently commented out.
src/pages/session/index.css, tailwind.config.js Updated class names and added new color definitions and theme extensions.

"With each commit, we hop ahead, 🐇
In the garden of code, where we tread. 🌱
From the roots of logic, branches spread, 🌳
Blooming solutions, beautifully bred. 🌼
So here's to the changes, big and small, 🥂
In the end, they matter to us all." 🌟

Tips ### Chat with CodeRabbit Bot (`@coderabbitai`) - Mention `@coderabbitai` in any *review comment* for bot assistance. - Note: Review comments are made on code diffs or files, not on the PR overview. ### Pause Incremental Reviews - Insert `@coderabbitai: ignore` in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
RSoulatIOHK commented 1 year ago

Full report

Scrolling

When looking at the full report, there are 2 horizontal scroll bars. Can we have at most 1 or even better 0 and line wrap the text?

There is a vertical scroll bar but it's kind of hidden on the far right and you need to use the right horizontal scroll bar to find it. Can we have it, on the right, at all times? I don't think people use the scroll bar to actually navigate but it will display that there is more information to see.

https://github.com/input-output-hk/dapps-certification-web/assets/117812549/50a6cfee-80c2-4db5-aa5e-8dd1563d4c8f

Display more information

It is unclear on this screen that you can click on the properties to show more information image Maybe just an arrow pointing downwards, with still the possibility to click anywhere you want to open the details.

"Purchase a certificate"

The "Purchase a Certificate (1 ADA)" should not be here for failed runs. image

Testing

Logs

The log opens all over everything. Can we have it open in between the timeline and the PBT results?

"Blank" page at the end of testing

At the end of testing, the timeline is removed and no testing results is displayed to the user: image

Could we have instead this kind of nice visualization, without the possibility to open the properties to see more detail results? image

Testing time still here from a precedent run

The testing time is still displayed from a previous run: image

Test again

Pressing test again gives you this screen: image

Not too sure what's going on, I think a new timeline has been fitted in the middle. The testing information on the left is empty. The previous "Full report" and "Download report" from the previous run should not be here anymore.

coderabbitai[bot] commented 1 year ago

Rate Limit Exceeded

You have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 29 minutes and 59 seconds before requesting another review.

To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR.

amnambiar commented 1 year ago

@RSoulatIOHK , Fixes are in place to re-review.

Full Report View

Scrolling

FIXED

Display more information - Maybe just an arrow pointing downwards, with still the possibility to click anywhere you want to open the details.

DONE ; added up/down clickable arrows to see the details, retaining click on row

The "Purchase a Certificate (1 ADA)" should not be here for failed runs.

FIXED ; This button will not be present if there any failures in unit tests or other tasks

Testing View

Logs - Can we have it open in between the timeline and the PBT results?

DONE

"Blank" page at the end of testing

FIXED ; the same table while the run in-progress will be shown

Testing time still here from a precedent run

FIXED ; all other issues related to state retention are as well fixed, PLT-7245 as well

Test again - bug

FIXED ; all states in New test and Test again are fixed

Others

Open

RSoulatIOHK commented 1 year ago

Still have the double scrolling bars and the vertical one is far away to see. image

When the repo doesn't exist, I get this Uncaught error image

Tool tip and check for repo accessibility are over one another: image

I know it's in the open issues but the progress % are not fine for most of the properties on some repo testing: image But is correct on the full report: image

I still see the previous testing time: image

Also the Github repo check is a bit agressive. I haven't managed to type the repo myself without triggering the check, could we just have it when you're no longer in the input bar?

All the rest looks fixed :)

amnambiar commented 1 year ago

@RSoulatIOHK , Thank you for the patience :bowing_woman:

Still have the double scrolling bars and the vertical one is far away to see.

Fixed for good now. Pardon :disappointed: Reason - mui-Container element had maxWidthdefaulted to 'lg'. I had changed it to 'xl' instead of setting it false. Is set to false now.

  • When the repo doesn't exist, I get this Uncaught error
  • Tool tip and check for repo accessibility are over one another:

Fixed

I still see the previous testing time:

Fixed :sweat_smile:

Github repo check - could we just have it when you're no longer in the input bar

Triggering the check on blur breaks at multiple scenarios, hence retained the check on type itself but with a lesser timeout than the older version of tool. It is more quicker, doesn't let PLT-7458 happen (Test btn will be disabled until the repo is accessible).

progress % issue

Raised PLT-7646 to track this

RSoulatIOHK commented 1 year ago

I still have the scroll bars :( If I put the window on my 4k screen, i don't have them but that might be a strong requirement for our users 😄 image (Let's put this one on the Tech debt if you want)

Rest is fine :) (apart from the tickets you have raised to track some of the bigger issues)

amnambiar commented 1 year ago

@RSoulatIOHK ,

Multiple scrollbars above 4k

I've added a fix for the multiple scrollbars. Hope it is resolved for you now. Anyhow I've raised a ticket PLT-7656. We can track it there.

Thanks!

amnambiar commented 1 year ago

Rechecked 'Testing' the form, timeline view, full report, state retention and it looks fine.

RSoulatIOHK commented 1 year ago

Apart from the issues in the timeline results when it ends (sometimes)

image

The rest looks good!