Skip to content

Conversation

@Xsy41
Copy link
Collaborator

@Xsy41 Xsy41 commented Feb 28, 2025

Brief Information

This pull request is in the type of (more info about types):

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • test

Related issues (all available keywords):

Details

image image

Checklist

Others

return null;
};

const waitForValidPopover = async (signal: AbortSignal): Promise<HTMLElement | null> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the signal parameter set when it is not used

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@wangyantong2000
Copy link
Collaborator

image This feature needs to be reviewed and done again.

@wangyantong2000 wangyantong2000 marked this pull request as draft March 1, 2025 04:27
@Xsy41
Copy link
Collaborator Author

Xsy41 commented Mar 4, 2025

Fixed the timer issue and removed unnecessary parameters. @wangyantong2000

@wangyantong2000
Copy link
Collaborator

image image

Only the homepage is in effect, other scenarios have not been considered yet

@Xsy41
Copy link
Collaborator Author

Xsy41 commented Mar 8, 2025

@wangyantong2000 Please review again

// if (existingPopover) {
// resolve(existingPopover as HTMLElement);
// return;
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're sure you don't need it here, you can delete it. Once ready, you can convert the draft PR to a formal PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@Xsy41 Xsy41 marked this pull request as ready for review March 8, 2025 12:07
@wangyantong2000 wangyantong2000 merged commit a53b027 into hypertrons:master Mar 8, 2025
3 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.

2 participants