openlcb / OpenLCB_Java

A git-managed copy of the SVN-based Java prototype implementation of OpenLCB. This implementation operates inside JMRI.
6 stars 9 forks source link

Move and animate "Refresh All" ... "Restore..." buttons #65

Closed dpharris closed 4 years ago

dpharris commented 7 years ago

Can the four buttons be moved to the always visible bar where the "Read All" button is?

Can the "Refresh All" and "Save Changes" buttons change colour to indicate something has changed?

balazsracz commented 7 years ago

David,

thanks for flagging this. I have already started working on it https://github.com/openlcb/OpenLCB_Java/commit/7f453a6fe20988fe619e238261da3792b02b7dfb#diff-db8575e4c24ce0b766d8d9f86883c5d4R134

There are some tricky issues around the nesting of JPanel objects and scrollbars that makes it more complicated than it sounds. Basically due to the split between the two codebases it was not possible to do this in one step and still get the interesting new features into 4.6:

So instead I focused on laying the foundation that will allow us to remove the Read All button in the first development release coming up. At that point I will be able to fix the bottom bar to be on the screen at all times.

On your other request (of coloring the buttons): I am not a fond of this idea. Generally coloring buttons breaks the look&feel of the UI and looks pretty kludgy therefore. It also is not guaranteed to work in different UIs, for example in the Max and GTK UI it would likely be broken.

What I can do is make the "Save changed" button be Disabled (grayed out) vs Enabled depending on whether you have any changed values. All look&feels have a native form of displaying a disabled button.

The "Refresh All" cannot be changed in a meaningful way, because it has no way of telling whether something changed on the node itself. "Refresh All" doesn't just discard all edited fields, it also reloads every field from the current value of the node. So pressing Refresh All makes sense even if you have not changed anything (but maybe someone else on a different computer did).

Balazs

dpharris commented 7 years ago

Balazs -- Thanks ... all good and acceptable explanations. You have made it so much better already that these changes can wait :-) David PS -- the load speed is great.

On Tue, Dec 13, 2016 at 11:17 AM, Balazs Racz notifications@github.com wrote:

David,

thanks for flagging this. I have already started working on it 7f453a6#diff- db8575e4c24ce0b766d8d9f86883c5d4R134 https://github.com/openlcb/OpenLCB_Java/commit/7f453a6fe20988fe619e238261da3792b02b7dfb#diff-db8575e4c24ce0b766d8d9f86883c5d4R134

There are some tricky issues around the nesting of JPanel objects and scrollbars that makes it more complicated than it sounds. Basically due to the split between the two codebases it was not possible to do this in one step and still get the interesting new features into 4.6:

  • the "Read All" button and the respective footer is rendered by jmri.jar, while the new buttons (Save changed, backup, restore, etc) are rendered by openlcb.jar.
  • there was no way to put the new openlcb buttons into the JMRI-rendered bar (no API for that)
  • openlcb.jar cannot influence the rendering that JMRI does
  • we cannot nest a scrollpane inside a scrollpane (it breaks scrolling by the mouse)
  • due to the code freeze coming up to the 4.6 prod release it was undesired to make large changes.

So instead I focused on laying the foundation that will allow us to remove the Read All button in the first development release coming up. At that point I will be able to fix the bottom bar to be on the screen at all times.

On your other request (of coloring the buttons): I am not a fond of this idea. Generally coloring buttons breaks the look&feel of the UI and looks pretty kludgy therefore. It also is not guaranteed to work in different UIs, for example in the Max and GTK UI it would likely be broken.

What I can do is make the "Save changed" button be Disabled (grayed out) vs Enabled depending on whether you have any changed values. All look&feels have a native form of displaying a disabled button.

The "Refresh All" cannot be changed in a meaningful way, because it has no way of telling whether something changed on the node itself. "Refresh All" doesn't just discard all edited fields, it also reloads every field from the current value of the node. So pressing Refresh All makes sense even if you have not changed anything (but maybe someone else on a different computer did).

Balazs

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openlcb/OpenLCB_Java/issues/65#issuecomment-266833732, or mute the thread https://github.com/notifications/unsubscribe-auth/AAg4SguJ4MS79frvew28E1o9Rh3CQLSAks5rHu89gaJpZM4LLQx5 .

balazsracz commented 7 years ago

These are now moved into a single button bar that is always visible.

The save changed still needs an disable/enable feature when something is dirty.

balazsracz commented 4 years ago

Duplicate of #147