-
Notifications
You must be signed in to change notification settings - Fork 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
Preserve property order in environment editor #1497
Preserve property order in environment editor #1497
Conversation
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Show resolved
Hide resolved
I'm not sure I added the dependency correctly - should |
It should be in both. |
Ah, @develohpanda it's because you need to add a Flow definition to the Here's a good example of one to take a look at: |
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.
PR looking great! Just a couple small comments and then one about whether or not we should have the checkbox to maintain order.
packages/insomnia-app/app/ui/components/editors/environment-editor.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/editors/environment-editor.js
Outdated
Show resolved
Hide resolved
@@ -30,10 +30,20 @@ class EnvironmentEditModal extends PureComponent { | |||
return; | |||
} | |||
|
|||
const environment = this._envEditor.getValue(); | |||
let patch; |
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.
Curious why you included the try/catch when there wasn't one before. Did you find an error case that wasn't handled?
@@ -342,9 +343,13 @@ class WorkspaceEnvironmentsEditModal extends React.PureComponent<Props, State> { | |||
return; | |||
} | |||
|
|||
let data; | |||
let patch; | |||
try { |
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.
Oh I see. Same thing here. I just checked the getValue
function and didn't realize it was doing JSON parsing. Makes sense now 👍
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Show resolved
Hide resolved
@gschier ready for review again :) |
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.
Awesome! Just gave it a spin locally and it works great 😄
Closes #1046.
Added an option to keep property order by consuming json-order. This is opt-in, thus disabled by default. This behavior is available to both the workspace environment editor, and the request group override environment editor.
Field names cannot begin by
$
or contain a.
(as part of thenedb
documentation).As a result, the map generation uses
&
as the prefix and~|
as the separator. These two options are parameters in theparse
andstringify
methods. An example map is shown below.