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

fix: persist interceptor state for logged out user #4060

Open
wants to merge 4 commits into
base: patch
Choose a base branch
from

Conversation

nivedin
Copy link
Member

@nivedin nivedin commented May 13, 2024

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

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

Copy link
Contributor

@JoelJacobStephen JoelJacobStephen left a 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.

image

@JoelJacobStephen JoelJacobStephen self-requested a review May 13, 2024 12:14
Copy link
Contributor

@JoelJacobStephen JoelJacobStephen left a 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?

@nivedin
Copy link
Member Author

nivedin commented May 13, 2024

@nivedin I noticed that the default interceptor is now set to None instead of Proxy when logged out. Is this intended?

In SH the default interceptor is "browser" ie "none", only for central platform the default option will be "proxy", the default value is set from individual platform ref

Copy link
Contributor

@JoelJacobStephen JoelJacobStephen left a 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!!

packages/hoppscotch-common/src/pages/settings.vue Outdated Show resolved Hide resolved
@nivedin nivedin changed the base branch from main to patch May 22, 2024 08:20
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.

None yet

4 participants