Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(LogSheet): Update UI of log download links #670

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

murilx
Copy link
Contributor

@murilx murilx commented Dec 11, 2024

  • Added a new function truncateUrl used to truncate the URL in the log viewer
  • Changed the title of the download log table to "Full logs"
  • Changed the messaged of the QuerySwitcher to "Download your logs here"
  • Changed the style of the log links, making their font size greater and their color blue with an underline hover effect

Closes #655

Visual Reference

Before
log_before

After
log_after

@murilx murilx self-assigned this Dec 11, 2024
dashboard/src/lib/string.ts Outdated Show resolved Hide resolved
@murilx murilx marked this pull request as draft December 11, 2024 20:12
@murilx murilx force-pushed the feat/improve-download-log-ui branch from ce8d2a8 to 97ea84c Compare December 12, 2024 14:25
@murilx murilx marked this pull request as ready for review December 12, 2024 14:25
Comment on lines 21 to 22
const protocolRe = /^\w+:\/\//;
const searchParamsRe = /\?.*$/;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const protocolRe = /^\w+:\/\//;
const searchParamsRe = /\?.*$/;
const protocolRegex = /^\w+:\/\//;
const searchParamsRegex = /\?.*$/;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let replacedUrl = url.replace(protocolRe, '');
replacedUrl = replacedUrl.replace(searchParamsRe, '');
const splittedUrl = replacedUrl.split('/');
const hostname = splittedUrl[0];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const hostname = splittedUrl[0];
const hostname = splittedUrl[0] ?? '';

typescript strict mode doesn't check array index

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm checking for undefined below

@@ -202,7 +205,7 @@ export const LogSheet = ({
skeletonClassname="h-[3rem]"
customError={
<div className="p-4 text-center">
This log url is not supported in the log viewer yet
You can download your logs here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now this wasn't on a message but please put this on a message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely forgot it, you are right

Copy link
Collaborator

@WilsonNet WilsonNet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some minor pointers

@murilx murilx force-pushed the feat/improve-download-log-ui branch from 97ea84c to f42d3d6 Compare December 12, 2024 18:17
Comment on lines 23 to 24
let replacedUrl = url.replace(protocolRegex, '');
replacedUrl = replacedUrl.replace(searchParamsRegex, '');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let replacedUrl = url.replace(protocolRegex, '');
replacedUrl = replacedUrl.replace(searchParamsRegex, '');
let replacedUrl = url.replace(protocolRegex, '').replace(searchParamsRegex, '');

@@ -202,7 +205,7 @@ export const LogSheet = ({
skeletonClassname="h-[3rem]"
customError={
<div className="p-4 text-center">
This log url is not supported in the log viewer yet
<FormattedMessage id="logSheet.logQueryCustomError" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message is for the log downloader, I don't think we should change it to a download info here; maybe change it above in the place of "Index of"

@murilx murilx force-pushed the feat/improve-download-log-ui branch 2 times, most recently from 20b3d45 to 38f4d90 Compare December 12, 2024 20:46
- Added a new function `truncateUrl` used to truncate the URL in the log
  viewer
- Changed the title of the download log table to "Full logs"
- Changed the messaged of the QuerySwitcher to "Download your logs
  here"
- Changed the style of the log links, making their font size greater and
  their color blue with an underline hover effect

Closes #655
@murilx murilx force-pushed the feat/improve-download-log-ui branch from 38f4d90 to a73b1a1 Compare December 12, 2024 21:08
Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

@murilx murilx merged commit 7d3dd57 into main Dec 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log Viewer: full log downloads does't looks like downloadable. Improve UI to make it clear.
4 participants