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

Top nav auth dropdown #15055

Merged
merged 6 commits into from
Oct 27, 2022
Merged

Top nav auth dropdown #15055

merged 6 commits into from
Oct 27, 2022

Conversation

philrenaud
Copy link
Contributor

Resolves #15051

Signed in:
image

Signed out:
image

Active:
image

@philrenaud philrenaud self-assigned this Oct 26, 2022
@github-actions
Copy link

github-actions bot commented Oct 26, 2022

Ember Asset Size action

As of 078379f

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +2.83 kB +473 B
nomad-ui.css +525 B +45 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented Oct 26, 2022

Ember Test Audit comparison

f-ui/sso 078379f change
passes 1427 1429 +2
failures 0 0 0
flaky 0 0 0
duration 10m 05s 637ms 10m 14s 706ms +09s 069ms

Copy link
Contributor

@ChaiWithJai ChaiWithJai left a comment

Choose a reason for hiding this comment

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

Left a few questions, but LGTM.

ui/app/components/global-header.js Outdated Show resolved Hide resolved
ui/app/components/global-header.js Outdated Show resolved Hide resolved
Comment on lines 67 to 70
@onChange={{action (queue
(fn (mut this.profileSelection))
this.profileSelection.action
)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

learning opportunity: This is unique. Is this an instance of why you want the ergonomic benefits of Ember 4.5? Why are you calling the mut helper here?

Are we using it for the child component PowerSelect to mutate the value of a parent GlobalHeader?

So you're creating a piped function that first mutates the value of the profileSelection to display to the user when the option is selected in the dropdown menu and then you're dispatching profileSelection.action to fire after we set the profileSelection value? Do I have that correct? And this is ergonomically better because now we're not writing that function in the backing component class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This lets us attach actions to the profileOptions objects themselves. PowerSelect doesn't give us a "currentlySelectedOption" entity by default in its template, so we use profileSelection to store that.

We don't do this to show the selection to the user; we always just show the word "Profile" to them.

@philrenaud philrenaud merged commit a50d93d into f-ui/sso Oct 27, 2022
@philrenaud philrenaud deleted the 15051-ui-sso-top-nav-auth-dropdown branch October 27, 2022 21:00
philrenaud added a commit that referenced this pull request Nov 3, 2022
* Basic dropdown styles

* Some cleanup

* delog

* Default nomad hover state styles

* Component separation-of-concerns and acceptance tests for auth dropdown

* lintfix
philrenaud added a commit that referenced this pull request Nov 28, 2022
* Top nav auth dropdown (#15055)

* Basic dropdown styles

* Some cleanup

* delog

* Default nomad hover state styles

* Component separation-of-concerns and acceptance tests for auth dropdown

* lintfix

* [ui, sso] Handle token expiry 500s (#15073)

* Handle error states generally

* Dont direct, just redirect

* no longer need explicit error on controller

* Redirect on token-doesnt-exist

* Forgot to import our time lib

* Linting on _blank

* Redirect tests

* changelog

* [ui, sso] warn user about pending token expiry (#15091)

* Handle error states generally

* Dont direct, just redirect

* no longer need explicit error on controller

* Linting on _blank

* Custom notification actions and shift the template to within an else block

* Lintfix

* Make the closeAction optional

* changelog

* Add a mirage token that will always expire in 11 minutes

* Test for token expiry with ember concurrency waiters

* concurrency handling for earlier test, and button redirect test

* [ui] if ACLs are disabled, remove the Sign In link from the top of the UI (#15114)

* Remove top nav link if ACLs disabled

* Change to an enabled-by-default model since you get no agent config when ACLs are disabled but you lack a token

* PR feedback addressed; down with double negative conditionals

* lintfix

* ember getter instead of ?.prop

* [SSO] Auth Methods and Mock OIDC Flow (#15155)

* Big ol first pass at a redirect sign in flow

* dont recursively add queryparams on redirect

* Passing state and code qps

* In which I go off the deep end and embed a faux provider page in the nomad ui

* Buggy but self-contained flow

* Flow auto-delay added and a little more polish to resetting token

* secret passing turned to accessor passing

* Handle SSO Failure

* General cleanup and test fix

* Lintfix

* SSO flow acceptance tests

* Percy snapshots added

* Explicitly note the OIDC test route is mirage only

* Handling failure case for complete-auth

* Leentfeex

* Tokens page styles (#15273)

* styling and moving columns around

* autofocus and enter press handling

* Styles refined

* Split up manager and regular tests

* Standardizing to a binary status state

* Serialize auth-methods response to use "name" as primary key (#15380)

* Serializer for unique-by-name

* Use @classic because of class extension
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants