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

Suggestion: lesser dialogs popping up when renaming #95

Open
StefanMaron opened this issue Feb 8, 2021 · 18 comments · May be fixed by #133
Open

Suggestion: lesser dialogs popping up when renaming #95

StefanMaron opened this issue Feb 8, 2021 · 18 comments · May be fixed by #133
Assignees
Labels
bug Something isn't working needs test defined needs testing Ready for testing version A To be picked up right away
Milestone

Comments

@StefanMaron
Copy link

Describe the bug
When working on a feature, I think it is common to do quite some changes at various places. For each change I will get asked for confirmation. While I think this is good when updating already existing features, for not braking stuff unintentionally, this disturbes the work flow on new features.

Suggestion
I think a switch to change to some kind of "Update Mode" would do the trick. Maybe with default from settings file. ("Update mode always enabled" true/false)
image

When update mode is active, all update confirmations will be automatically answered with yes.

@lvanvugt
Copy link
Contributor

lvanvugt commented Feb 9, 2021

Commit was wrongly assigned to this issue. Should have been linked to #94.

@lvanvugt lvanvugt added this to the beta release milestone Feb 9, 2021
@lvanvugt lvanvugt added the version A To be picked up right away label Feb 9, 2021
@lvanvugt lvanvugt changed the title Problems with renaming Suggestion: lesser dialogs popping up when renaming Feb 9, 2021
@lvanvugt
Copy link
Contributor

lvanvugt commented Feb 9, 2021

Seems less annoying with "atddTestScriptor.removalMode": "No confirmation, but removal", then you only get a dialog on the scenario of given rename.
However, a switch to disable all dialogs might make sense. @DavidFeldhoff for you to pick this up.

@lvanvugt
Copy link
Contributor

lvanvugt commented Feb 18, 2021

Solve it by adding a setting that controls turning of all dialogs (update, remove ...) -> @DavidFeldhoff
Add a UI element that sets it -> @martonsagi (sends info to backend to handled)

@DavidFeldhoff
Copy link
Collaborator

I added a setting "showConfirmations" which is true by default.

@DavidFeldhoff DavidFeldhoff removed their assignment Mar 11, 2021
@DavidFeldhoff
Copy link
Collaborator

Backendcall: Set/GetConfiguration. Implemented by me, called by Marton

@lvanvugt
Copy link
Contributor

What's next to do here?

@martonsagi
Copy link
Collaborator

UI toggle has been implemented.
@DavidFeldhoff : I could not find the backend calls, so I have added them to the WebPanelCommandService class as skeleton code just to make it work with the UI. Actual setting handling needs to be finished on you side.

ATDD_UpdateMode

@martonsagi martonsagi removed their assignment Apr 1, 2021
martonsagi added a commit that referenced this issue Apr 2, 2021
@DavidFeldhoff
Copy link
Collaborator

The setting is set now, but because of some reason the setting isn't read correctly on startup here
https://github.com/fluxxus-nl/ATDD.TestScriptor.VSCodeExtension/blob/master/web-ui/src/resources/elements/toolbar.ts#L21

Can you please have a look at it, @martonsagi ?

@lvanvugt
Copy link
Contributor

lvanvugt commented Apr 8, 2021

Consider to rename No confirmation to Disable confirmations.

@lvanvugt lvanvugt assigned lvanvugt and unassigned martonsagi Apr 8, 2021
@lvanvugt lvanvugt added the needs testing Ready for testing label Apr 8, 2021
@DavidFeldhoff DavidFeldhoff assigned martonsagi and unassigned lvanvugt Apr 8, 2021
@DavidFeldhoff
Copy link
Collaborator

As I'm working with it: I think this shouldn't be mostly a workspace setting, but a user setting.
Currently, If I change this setting in the ATDD page, it shows up directly in my git-changes as the workspace file is changed. But I will typically never commit that change to the workspace file as it is a preference of me that I don't want the confirmations to show up (=user preference -> user setting).
IF it is set on workspace level manually as well, then we have to consider that of course, but if it's not set, I'd set it on user level instead of on workspace level. I think it's better explained in a table :)

Setting is set in user settings Setting is set in workspace settings ATDD updates/inserts setting in
user settings
x user settings
x workspace settings
x x workspace settings

How do you think about that @lvanvugt, @StefanMaron?

@StefanMaron
Copy link
Author

At least for me it does not matter where to set up stuff, as long as I am able to set up at all

@lvanvugt
Copy link
Contributor

UI toggle has been implemented. @DavidFeldhoff : I could not find the backend calls, so I have added them to the WebPanelCommandService class as skeleton code just to make it work with the UI. Actual setting handling needs to be finished on you side.

ATDD_UpdateMode

@martonsagi, I do not see the button. Probably I do not have the latest build. Probably need to learn again were to find it. ;-)

@lvanvugt lvanvugt modified the milestones: beta release, v1 Nov 26, 2022
@lvanvugt lvanvugt assigned lvanvugt and unassigned martonsagi Nov 27, 2022
@lvanvugt
Copy link
Contributor

lvanvugt commented Nov 27, 2022

It was not yet available in the latest published version (0.9.218), but it was in a later internal build https://fluxxus.visualstudio.com/TestScriptor/_build/results?buildId=232.
So, needs to be tested first.

@lvanvugt
Copy link
Contributor

lvanvugt commented Jan 23, 2023

As I'm working with it: I think this shouldn't be mostly a workspace setting, but a user setting. Currently, If I change this setting in the ATDD page, it shows up directly in my git-changes as the workspace file is changed. But I will typically never commit that change to the workspace file as it is a preference of me that I don't want the confirmations to show up (=user preference -> user setting). IF it is set on workspace level manually as well, then we have to consider that of course, but if it's not set, I'd set it on user level instead of on workspace level. I think it's better explained in a table :)

@DavidFeldhoff, is it right that this whole consideration has not been implemented?

Testing it right now it seems always to be updating the workspace setting.json. Next to that when "atddTestScriptor.showConfirmations": false it seems that when removing only first level removal is done, i.e., when removing a GIVEN only the GIVEN tag and related code line is removed not the helper method called from that line.

@lvanvugt lvanvugt assigned DavidFeldhoff and unassigned lvanvugt Feb 17, 2023
@lvanvugt
Copy link
Contributor

  1. As discussed I would expect this setting to be added to the user settings
  2. And please test the issue on the removal of the helper method, also.

@lvanvugt
Copy link
Contributor

lvanvugt commented Feb 19, 2023

  1. As discussed I would expect this setting to be added to the user settings

Solved by previous commit

  1. And please test the issue on the removal of the helper method, also.

Not an issue in this context (when testing on f.e. codeunit 50100 "TestObjectFLX" it shows to work fine. However when testing on a more "complex" codeunit like codeunit 50105 "TestObjectWithTwoFeaturesFLX" removing a Given or Then in TestFunction2 where Given/Then tag does not 100% related to the name of the helper method it is not remove. Need to tackle this in a separate issue (new Use Case). Clearly described the behaviour here: #131.

@lvanvugt lvanvugt assigned lvanvugt and unassigned DavidFeldhoff Feb 19, 2023
@lvanvugt
Copy link
Contributor

Consider to rename No confirmation to Disable confirmations.

@martonsagi, could you please implement this proposal?

@christianbraeunlich christianbraeunlich linked a pull request Sep 16, 2023 that will close this issue
@christianbraeunlich
Copy link
Contributor

christianbraeunlich commented Sep 16, 2023

I haven't read all the comments carefully enough and although, I like the renaming from "No" to "Disable", I wouldn't want to see a button here. I would just like to use a setting in vs code for that.

And if we're worried about the devs not noticing, we could provide a wizard right when the extension is installed or a small dialog box in the bottom right corner where the devs can choose to disable confirmations.

The project is most likely backed up by version control anyway, so why even bother about confirmations in the first place? From my pov, confirmation is when the changes committed to the branch are merged into the default/release branch, usually via PR. Forgive my naive standpoint. Mostly just use the extension as an end user and don't bother about how it's done technically in the background, but their might be a reason to ask for confirmations, that's why I'm opening this as for discussion

@lvanvugt is there anything else left to do besides the renaming of the button?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs test defined needs testing Ready for testing version A To be picked up right away
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants