Fix cancel button #40
Conversation
imobachgs
force-pushed
the
hash-collections
branch
from
f7dd5af
to
edb8bc9
Compare
imobachgs
changed the base branch from
hash-collections
to
salt-forms-support-ng
| @form_data_instances.pop unless @form_data_instances.size <= 1 | ||
| end | ||
|
|
||
| # Clears the last backup if it exists |
There was a problem hiding this comment.
It removes the previous backup if I get correctly, isn't it?
|
|
||
| # Clears the last backup if it exists | ||
| def remove_backup | ||
| @form_data_instances.delete_at(-2) if @form_data_instances.size > 1 |
There was a problem hiding this comment.
delete_at(-2) ? This looks very strange. Before this PR, FormControllerState is just a stack of triplets, very simple. Now I don't understand it anymore.
What are the invariants of its state?
There was a problem hiding this comment.
form_data_instances keep a set of "snapshots" of the form state. Everytime you open a nested form, the current form values are saved in the stack. So if you decide to cancel the form, we just rollback to the previous version (removing the top of the stack).
Thinking about it, it might be easier to understand if we track the current value in a different instance variable (like @form_data) and keep the backups in a separate one (@form_data_backups).
| # This class is responsible for reading the form data from its definition and a data pillar. | ||
| # | ||
| # The format used in the pillar is slightly different from the internal representation of this | ||
| # module, so this class takes care of the conversion. However, the initial intentation is to not |
There was a problem hiding this comment.
Huh, do we really have a representation of the data that is different from the pillar contents?
If so, this comment should explain the difference.
There was a problem hiding this comment.
Ah, like, we use FormData which has a #to_h method for the pillar, is that it?
There was a problem hiding this comment.
Ok, I will. Basically, keeping our own organization (especially when it comes to collections) makes the implementation easier. For instance, the collection widget does not need to know anything about the pillars format (hash and array based collections). Additionally, I felt that the pillar organization was leaking to our UI (and I think that's wrong).
src/lib/y2configuration_management/salt/form_controller_state.rb
Outdated
Show resolved
Hide resolved
Co-Authored-By: imobachgs <imobachgs@gmail.com>
imobachgs
force-pushed
the
fix-cancel-button
branch
from
171b3bc
to
c455652
Compare
This PR fixes the behaviour of the
Cancelbutton. However, we might need to rebase this branchonce that #39 is merged.