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

Added properties to collections #718

Merged

Conversation

thehorse2000
Copy link

@thehorse2000 thehorse2000 commented Oct 21, 2023

Description

This change introduces storing properties for collections.
Currently, two properties are stored in collections:

  1. BASE URL which will be the initial default value for any new request within that collection
  2. Default Request Type also sets initially the request type for new requests.

This is very helpful to avoid repetition of entering the same URL over and over again.
This also opens up more space for future customizations for the collections.

The collection properties are stored in the bruno.json file of the collection

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

For previous comments of this PR. See #707

image

Monosnap.screencast.2023-10-21.20-47-15.mp4

@helloanoop
Copy link
Contributor

@thehorse2000 Nice!

Fully onboard with supporting this. I

I'd like to recommend a few changes.
The properties modal was kind of inspired by how we have properties of a file or folder in any OS.
I recommend putting this inside the collection settings and calling the Tab Presets
The config can be stored inside bruno.json under the key presets

{
  "presets": {
     "defaultUrl": "",
     "defaultRequestType": "",
  }
}

image

@thehorse2000
Copy link
Author

Hey, Thanks for the info. I actually didn't know there was a collections settings area 😿
I think that is because we can only access it from requests.
I believe it would be better if we could access these settings from the menu of the collection (like properties)
But maybe that is for another PR 😄

@thehorse2000
Copy link
Author

New Changes

  • Changed "properties" to "presets" in bruno.json file
  • Added Presets tab to collection settings
  • Returned collection properties modal to its previous state.

image

Monosnap.screencast.2023-10-22.15-33-14.mp4

Copy link
Contributor

@helloanoop helloanoop left a comment

Choose a reason for hiding this comment

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

@thehorse2000 Nice work. I have a couple more comments. Once these addressed, I feel this should be good to merge.

@@ -94,6 +94,28 @@ const registerRendererEventHandlers = (mainWindow, watcher, lastOpenedCollection
}
});

// update collection properties
Copy link
Contributor

Choose a reason for hiding this comment

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

@thehorse2000 You don't need a new electron event. There is already one which handles the updates for Bruno Config file.

https://github.com/usebruno/bruno/blob/main/packages/bruno-app/src/components/CollectionSettings/index.js#L34-L42

Can you update the code to use that?

@@ -114,6 +118,9 @@ const CollectionSettings = ({ collection }) => {
<div className={getTabClassname('headers')} role="tab" onClick={() => setTab('headers')}>
Headers
</div>
<div className={getTabClassname('presets')} role="tab" onClick={() => setTab('presets')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets place this tab after Tests and before Proxy

@martinsefcik
Copy link
Contributor

@thehorse2000 @helloanoop Probably this Settings tab for collection could be opened by clicking on collection name in sidebar, the same way how it is working in Postman. It could be then also used to switch quickly environment, because now if I want to run whole collection or folder I have to unfold many subfolders find first request in collection open it and then I can switch environment which is not very user friendly.

Also this Collection properties in modal and Collection settings in tab could be merged to Collection tab. It doeas not make sense to me to have two places in app for similar things.

@helloanoop
Copy link
Contributor

Probably this Settings tab for collection could be opened by clicking on collection name

Agreed.

Also this Collection properties in modal and Collection settings in tab could be merged to Collection tab. It doeas not make sense to me to have two places in app for similar things.

Agreed. Thinking of a new Tab called Info which will display the same stuff that we display in Properties

@thehorse2000
Copy link
Author

@helloanoop Resolved the required changes.
I do also agree with you and @martinsefcik opinions on making the collections settings page more accessible.
So I will work on it, if you guys don't mind.
Will create a new PR for it and mention you

@helloanoop helloanoop merged commit 7367972 into usebruno:main Nov 19, 2023
2 checks passed
@helloanoop
Copy link
Contributor

Nice work @thehorse2000 !

PR Merged.

@helloanoop
Copy link
Contributor

Probably this Settings tab for collection could be opened by clicking on collection name in sidebar

@martinsefcik This is done.

Also this Collection properties in modal and Collection settings in tab could be merged to Collection tab. It doeas not make sense to me to have two places in app for similar things.

This is also done. See #989
I also removed Properties in the collection dropdown and added Settings

It could be then also used to switch quickly environment, because now if I want to run whole collection or folder I have to unfold many subfolders find first request in collection open it and then I can switch environment which is not very user friendly.

@martinsefcik I didn't understand. Can you explain this?
You should be able to select any request and switch the env on the top-right corner right ?

@martinsefcik
Copy link
Contributor

martinsefcik commented Nov 19, 2023

Probably this Settings tab for collection could be opened by clicking on collection name in sidebar

@martinsefcik This is done.

Thanks.

It could be then also used to switch quickly environment, because now if I want to run whole collection or folder I have to unfold many subfolders find first request in collection open it and then I can switch environment which is not very user friendly.

@martinsefcik I didn't understand. Can you explain this? You should be able to select any request and switch the env on the top-right corner right ?

@helloanoop If you have for example the following collection structure:

- folder 1
  - folder 1.1
    - folder 1.1.1
      - folder 1.1.1.1
        - request 1
        - request 2
        - ...
- folder 2
  - folder 2.1
    - folder 2.1.1
      - folder 2.1.1.1
        - request a
        - request b
        - ...

then you have to unfold first 4 folders to get inside folder with request, then open request and the switch environment.

So I meant this that it isn't/wasn't very user friendly.

@martinsefcik
Copy link
Contributor

Probably this Settings tab for collection could be opened by clicking on collection name in sidebar

@martinsefcik This is done.

@helloanoop I tested 1.2.0 and now I realized that you put this open collection settings action on click event at different place and it does not help much in scenario I described in previous comment (#718 (comment))
image

Also wouldn't be better to rename this setting tab name from Collection to Settings?
For me collection area is the whole right part (excluding sidebar) of the application screen. And tab itself contains collection settings. So to have Collection in tab name is a bit confusing at least for me :)
image

@garposmack
Copy link

I loved this new feature.
It's possible to go one step further, and have folders inside a collection to have the same settings and properties as a collection?

@sanjai0py sanjai0py mentioned this pull request Jul 2, 2024
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