Skip to content

feat: add port sharing frontend #12119

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

Merged
merged 16 commits into from
Feb 20, 2024
Merged

feat: add port sharing frontend #12119

merged 16 commits into from
Feb 20, 2024

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented Feb 13, 2024

The following improvements can be made in future PRs:

  • Lack of loading states on share level changes and deletions make those actions feel laggy
  • Lack of scrollbars means the popover eventually gets too big to fit on the screen with enough items

Template Setting:
image

PortsPopover:
Screenshot 2024-02-20 at 12 43 06 PM

Template Max Sharing Level Owner:
Screenshot 2024-02-16 at 2 11 41 PM

Template Max Sharing Level Authenticated:
Screenshot 2024-02-16 at 2 18 47 PM
Screenshot 2024-02-16 at 2 18 55 PM

Experiment Not Enabled:
Screenshot 2024-02-16 at 2 11 33 PM

@f0ssel f0ssel changed the title feat: Add port sharing frontend feat: add port sharing frontend Feb 16, 2024
@f0ssel f0ssel marked this pull request as ready for review February 16, 2024 19:23
@f0ssel
Copy link
Contributor Author

f0ssel commented Feb 16, 2024

@BrunoQuaresma I'm going to continue working on adding tests for this feature, but I think the feature is in a good spot to get an initial review. Thanks

@f0ssel f0ssel requested a review from BrunoQuaresma February 16, 2024 19:24
Copy link

github-actions bot commented Feb 16, 2024


✔️ PR 12119 Updated successfully.
🚀 Access the credentials here.

cc: @f0ssel

@BrunoQuaresma
Copy link
Collaborator

Minor comment related to the template settings design: I think the select, label, and text helper should look like the "Days with required stop" field:
Screenshot 2024-02-20 at 09 26 48

Wdyt?

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

So far, it looks pretty good. I left minor comments.

@f0ssel
Copy link
Contributor Author

f0ssel commented Feb 20, 2024

Wdyt?

Totally, I think I was looking for a good example to copy, thanks.

@f0ssel f0ssel requested a review from BrunoQuaresma February 20, 2024 18:23
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

Good work! 👍

@f0ssel f0ssel merged commit b342bd7 into main Feb 20, 2024
@f0ssel f0ssel deleted the f0ssel/shared-ports-frontend branch February 20, 2024 18:26
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants