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

fix(logs): feedback #4 #2269

Merged
merged 5 commits into from
Jun 6, 2024
Merged

fix(logs): feedback #4 #2269

merged 5 commits into from
Jun 6, 2024

Conversation

bodinsamuel
Copy link
Collaborator

@bodinsamuel bodinsamuel commented Jun 5, 2024

Describe your changes

Changes:

  • Fixes NAN-1109 - Period not syncing from querystring to state
  • Fixes NAN-1102 - Missing scrollbar for message payload
  • Fixes NAN-1101 - Fix height and scrollbar for large operation payload
  • Fixes NAN-1104 - Preset period should always be live (last N <minutes|hours|day>
  • Fixes NAN-1107 - Preset should be "Last 24 hours" by default
  • Fixes NAN-986 - Change all links to new UI
  • Contributes to NAN-1097 - Rename old page to Activity

Bonus

  • Liveness is now synced in the URL to be able to share url. Period will be sliding by default when live is true. Meaning if you press "last 5minutes" it will follow Date.now instead of drifting.
  • Add an Info bloc in the old UI

@bodinsamuel bodinsamuel self-assigned this Jun 5, 2024
@@ -120,11 +121,17 @@ export default function Syncs({ syncs, connection, provider, reload, loaded, syn
setShowTriggerFullLoader(false);
};

const renderBubble = (bubbleType: ReactNode, sync: SyncResponse) => {
const RenderBubble = ({ sync, children }: { sync: SyncResponse; children: ReactNode }) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed that to a proper component for clarification

@bodinsamuel bodinsamuel marked this pull request as ready for review June 5, 2024 13:22
@khaliqgant
Copy link
Member

Will review this first thing tomorrow

@@ -265,7 +266,12 @@ We could not retrieve and/or refresh your access token due to the following erro
<ErrorCircle />
<span className="ml-2">There was an error refreshing the credentials</span>
<Link
to={`/${env}/activity?activity_log_id=${connectionResponse.errorLog.activity_log_id}&connection=${connectionResponse.connection.connection_id}&date=${getSimpleDate(connectionResponse.errorLog?.created_at?.toString())}`}
to={getLogsUrl({
Copy link
Member

Choose a reason for hiding this comment

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

When clicking from the connections page I get this

image

which in of itself the x looks misaligned

Copy link
Member

Choose a reason for hiding this comment

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

bad-link.mov

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

weird, thanks for testing that looking into it

Copy link
Collaborator Author

@bodinsamuel bodinsamuel Jun 6, 2024

Choose a reason for hiding this comment

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

I don't have any issue on my side, however I noted the Failed state does not appear if the schedule is paused

Screen.Recording.2024-06-06.at.11.36.21.mov

Copy link
Member

@khaliqgant khaliqgant left a comment

Choose a reason for hiding this comment

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

Code looks good, few small issues mentioned inline. One small nit as well

image

@bodinsamuel
Copy link
Collaborator Author

Code looks good, few small issues mentioned inline. One small nit as well

Fixed

@bodinsamuel bodinsamuel merged commit 911ed9d into master Jun 6, 2024
21 checks passed
@bodinsamuel bodinsamuel deleted the fix/logs-feedback-4 branch June 6, 2024 10:19
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.

3 participants