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

spin modified but not applied rules #1

Open
12 of 13 tasks
kulikov-a opened this issue Feb 27, 2021 · 12 comments
Open
12 of 13 tasks

spin modified but not applied rules #1

kulikov-a opened this issue Feb 27, 2021 · 12 comments

Comments

@kulikov-a
Copy link
Owner

kulikov-a commented Feb 27, 2021

Based on idea from the forum
https://forum.opnsense.org/index.php?topic=21634.msg102526#msg102526
Trying to add a changes preview before applying

  • spin if act or log toggled
  • keep spin until applying even when changing interfaces and refreshing pages
  • spin on bulk enable\disable
  • spin arrow icon on rule order change
  • spin pencil icon if rule changed by "Edit" or Added
  • handle rule deletion
  • accidentally lose part of the work done
  • restore rules order changes procedures
  • restore rule edit procedures
  • restore rule deletion procedures
  • remember and fix IE compatibility
  • move by-interface sorting from rule edit form to Apply button
  • cleanup
toggle_spin.mp4
@opnsenseuser
Copy link

👍

@kulikov-a
Copy link
Owner Author

updated

spin2.mp4

@kulikov-a
Copy link
Owner Author

kulikov-a commented Mar 2, 2021

@opnsenseuser
I'm starting to doubt @fichtner and @AdSchellevis would agree to such a change: quite a lot of code has already been added to track rule movement. it takes even more to track the creation and cloning of rules (and without this, it is impossible to ensure the persistence of the display of changes if multiple changes and additions of rules are made before applying the changes).
I don't know if it makes sense to continue

@opnsenseuser
Copy link

at that time i also wanted to use a moving icon for my sidebar to switch the sidebar. I thought if truenas used something like that, then they would accept it. as you can see unfortunately not. it became a static icon. that's why i decided to cross out the rule for the time being. I am not dependent there and there is an improvement and more recognizable.

@kulikov-a
Copy link
Owner Author

it's always better to try and fail than not try at all)
I'll try to finish what I started and see what comes of it
Thanks!

@opnsenseuser
Copy link

you did a great job by the way. i hope they will take it 👍

@AdSchellevis
Copy link

@kulikov-a we're indeed not a fan of large codeblocks on legacy code, ideally these pages still have to be converted to mvc, which will be more complicated with every addition we put in place, certainly if it doesn't fit the existing components we use in the mvc parts of our system.

@kulikov-a
Copy link
Owner Author

@AdSchellevis
thanks for joining the conversation!
now all the code is client-side. and of course will require constant tracking of changes in the backend code. Do I understand you correctly that for the correct implementation of such changes, it is necessary to expand the API to return some difference between the data in the config and the data in the rules.debug?

@AdSchellevis
Copy link

@kulikov-a the code "in the middle"[1] has been refactored already, the gui (and missing api) parts are the issue. but since these pages are a bit too complicated to fit in our standard bootgrid and the underlaying model isn't migrated, it's a lot of work to do this properly. eventually we will have to do this, which is also the reason why the firewall api plugin exists, which could serve more or less as the target platform.

If changes add a lot of value, we do tend to include them and cleanup the legacy php pages, but with every addition we do there, it's getting harder to reach the actual goal....

This year I don't expect we will replace the gui, but we are being careful what we do add to the legacy components (less logic and features that are more or less like the new components [bootgrid for example], help to make another step, options for which we don't really have new alternatives are less attractive)

Some optical improvements look great in the beginning, but cause us quite some headaches later on....

[1] https://github.com/opnsense/core/tree/master/src/opnsense/mvc/app/library/OPNsense/Firewall

@kulikov-a
Copy link
Owner Author

@AdSchellevis thanks for the clarifications! it seems the general idea has become a little clearer. The question is, is it really worth using the MVC for optical enhancements of this kind (wasting server resources on spinning icons)?
I think that optical enhancements will still located in the View part? And to spend server resources on the Controller\Model parts on tracking rule changes before applying just for the sake of spinning icons, it seems to me also not very reasonable.
my point is: if rules changes tracking (via localStorage) and applying classes on the client side turns out to be quite elegant and compact (I'm sure that the amount of code may not be so big, it's more in my programming level. I will try to optimize) can it be included and then adapted as legacy code will move to the MVC architecture?

@AdSchellevis
Copy link

@kulikov-a I'm not saying we won't add functions to the legacy components, just managing expectations. If you open a PR we can always discuss, but it might not make it to the project for various reasons.

@kulikov-a
Copy link
Owner Author

@AdSchellevis
got it, thanks! I will try to make it not cumbersome.
thanks again for your time

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

No branches or pull requests

3 participants