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: new policy view with login CTA #186

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Conversation

kopecs
Copy link
Contributor

@kopecs kopecs commented Nov 22, 2024

The policy view is blank on logging in. This still version of this commit needs some imports updated. Planning on going through and reorganizing imports generally / improving autoformatting and then rebasing probably.

PR checklist:

  • Purpose of the code is evident to future readers
  • Tests included or PR comment includes a reproducible test plan
  • Documentation is up-to-date
  • A changelog entry was for any user-facing change
  • Change has no security implications (otherwise, ping security team)

If you're unsure about any of this, please see:

Logged out Logged in
image image

The policy view is blank on logging in. This still version of this commit needs
some imports updated. Planning on going through and reorganizing imports
generally / improving autoformatting and then rebasing probably.
@kopecs kopecs requested a review from ajbt200128 December 2, 2024 19:18
Comment on lines +256 to +259
// TODO(cooper): This is an ugly hack to allow for easily updating the login
// status. It is rather fragile and we should instead do this by cleaning up
// the LSP extensions we use for login. However, this lets us ensure we have
// reasonably up-to-date UI state until then.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be updated per our discussion but I would like to merge this in the interim.

@kopecs kopecs marked this pull request as ready for review December 2, 2024 19:18
@ajbt200128
Copy link
Contributor

screenshots?

return element;
}

getChildren(
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the source for this data?

Copy link
Contributor Author

@kopecs kopecs Dec 2, 2024

Choose a reason for hiding this comment

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

Oh whoops; sorry there should be placeholder text (essentially just "you're logged in") here in this version. I have more in a future (wip) PR.

@kopecs
Copy link
Contributor Author

kopecs commented Dec 4, 2024

I'm going to move this back to draft while I update the login flow stuff we discussed since it is causing some cases where the UI would be stale here for now.

@kopecs kopecs marked this pull request as draft December 4, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants