-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: persist interceptor state for logged out user #4060
base: patch
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI is inconsistent between the interceptor component and the rest of the settings page. The font is quite small and there is unnecessary left padding on the component. I think we can split the interceptor component into two, isolating just the radio buttons into a new component and reuse that for the settings page and interceptor popup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nivedin I noticed that the default interceptor is now set to None instead of Proxy when logged out. Is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left a suggestion. You can take the call on whether we need to display the selected interceptor again on top. Rest LGTM!!
156b1cb
to
850954e
Compare
Closes HFE-508 #4059
Description
This PR fixes the the bug where a logged out user's interceptor option did not get persisted while refreshing. Also added interceptor option in settings page.
Checks