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: persist line wrap setting #3647

Conversation

ajmalmohad
Copy link
Contributor

@ajmalmohad ajmalmohad commented Dec 13, 2023

Persisted line wrap setting across various fields

Closes #3641

Description

Added settings for wrap line toggle for editor fields

HTTP: Request Body, Response Body, Headers, Parameters, URL Encoded, Pre-request, Test
GraphQL: Query, Response Body, Headers, Variables, Schema
Realtime: Communication Body, Realtime Log
Others: Import cURL, Code Generation, Cookie

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

Additional Information

persist.mp4

Copy link
Member

@jamesgeorge007 jamesgeorge007 left a comment

Choose a reason for hiding this comment

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

I was wondering if we would want to persist line wrap preference (communication message editor instance/ response logs) across real-time protocols? It was appropriate for REST/GQL requests, being the same entity, and since they persisted across tabs. But shouldn't we keep that distinction and scope the settings in the case of WebSocket, SSE, etc, being separate protocols?

Also, for the individual log entries, updating the line wrap preference next to one updates it across, which might not be ideal since having a line wrap toggle next to each log item conveys that it is specific to the respective entry. It should be moved to the Log section title if keeping this behavior.

Screen.Recording.2024-01-02.at.12.25.14.PM.mov

cc @liyasthomas

@ajmalmohad
Copy link
Contributor Author

I was wondering if we would want to persist line wrap preference (communication message editor instance/ response logs) across real-time protocols? It was appropriate for REST/GQL requests, being the same entity, and since they persisted across tabs. But shouldn't we keep that distinction and scope the settings in the case of WebSocket, SSE, etc, being separate protocols?

Also, for the individual log entries, updating the line wrap preference next to one updates it across, which might not be ideal since having a line wrap toggle next to each log item conveys that it is specific to the respective entry. It should be moved to the Log section title if keeping this behavior.

Screen.Recording.2024-01-02.at.12.25.14.PM.mov
cc @liyasthomas

It's seems rather inconvenient that the preference for wrap lines extends across individual log entries. It might be better to skip saving this setting for log entries.

Also I think it would be more desirable to have the line wrap setting for individual entries rather than a universal setting applied to everything by placing it within the section title.

We might also have to maintain the distinction and scope the settings separately for WebSocket and Socket.IO protocols.
Should we consider adding an additional property to the realtimeCommunication component to distinguish its usage?

@jamesgeorge007
Copy link
Member

@ajmalmohad, I discussed this with @liyasthomas. Let's not persist the line wrap preference for the realtime view and keep the behavior as earlier for the communication message editor instance and log entries where the preference is kept in the local component state.

@ajmalmohad
Copy link
Contributor Author

@jamesgeorge007 I've reverted the changes made to the realtime view components and removed the associated settings.

@jamesgeorge007 jamesgeorge007 changed the base branch from main to release/2023.12.3 January 22, 2024 11:13
@AndrewBastin AndrewBastin changed the base branch from release/2023.12.3 to release/2023.12.4 February 5, 2024 17:43
@AndrewBastin AndrewBastin merged commit 47226be into hoppscotch:release/2023.12.4 Feb 9, 2024
1 check passed
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.

[feature]: Persist wrap lines settings
3 participants