JFXtras / jfxtras

A supporting library for JavaFX, containing helper classes, extended layouts, controls and other interesting widgets.
http://jfxtras.org
Other
599 stars 123 forks source link

selectOneAppointmentCallback and appointmentBodyPaneMap #52

Closed daviddbal closed 8 years ago

daviddbal commented 8 years ago

allows selecting just one appointment to run a callback, which opens a popup

populates a map in Agenda when appointment body panes are made. Allows callbacks to match up Appointments to the body pane to position popups and set the owner node.

daviddbal commented 8 years ago

I think any skin will render Appointments as Panes. If that is the case, then why is it a problem to collect that information for Agenda?

Nevertheless, I need someway to get the rendered Panes in Agenda for the popups. The coordinates are not sufficient. The popup show method requires an owner node. Do you have a different idea to get me the Pane another way?

daviddbal commented 8 years ago

I did two changes in this PR (technically three, but one is just variable renaming).

One is for a select one appointment callback. Did you intend to reject that one too?

tbee commented 8 years ago

A simple list of appointments in a day does not use a pane-per-appointment. If you want something that is graphical, you really need to ask the skin directly, Agenda will never expose rendering details.

The appointment changed PR has been merged already. I added some additional calls, and a test. I'm refining it still, because a better version of the test is failing ATM. It may have something to do with the equals you implemented.

tbee commented 8 years ago

Agenda is a node on its own, isn't it? Or else the skin should be?

daviddbal commented 8 years ago

Yes, Agenda is a node, but not the right node. I tried it. Using it won't place the popup correctly. Also, the hide features won't work because Agenda doesn't lose focus when you click elsewhere on the calendar. Hide on escape doesn't work either.

daviddbal commented 8 years ago

I didn't ask about the changed appointment PR. I know that is merged.

I asked about the select one appointment callback. It's new. It was bundled with the appointment-pane map that you rejected. The select one appointment callback fires when the user clicks on just one appointment. You previously mentioned this kind of a feature in an email.

tbee commented 8 years ago

I did not see that one commit on selecting one appointment. That seems ok... Hmmm, please explain why it is needed.

daviddbal commented 8 years ago

Popular calendar programs, such as Google and Yahoo web calendars, have a popup appear when an appointment is clicked on. The popup shows some detailed information and allowing the user to choose to edit or delete it. I want to provide the same capability.

daviddbal commented 8 years ago

I don't see how I can ask the skin for the graphical information I want. I can get a list of all the children nodes that represent the Appointments, but how do I determine which is the one that was clicked on? I could use the ID, but I don't like requiring all skins to use the same ID naming conventions.

I am following the same pattern of making a popup as you used for the AppointmentMenu. You pass the AppointmentAbstractPane to the AppointmentMenu as the owner node. Because that code in imbedded in the skin you have that information. I just want the same information outside the skin.

I have an idea. Make a new class that acts like a structure containing two fields - Appointment and Pane. Replace the passed argument in the callbacks with the new class.

Another option is to abandon popups and just use new scenes. Still, even with new scenes it's nice to have coordinates or they just appear in the middle of the screen.

What do you think?

daviddbal commented 8 years ago

Perhaps the Pair<K,V> class could contain the Appointment and Pane for the callback.

tbee commented 8 years ago

I remember what I suggested concerning that popup:

Agenda will never expose information about how it is rendered.

daviddbal commented 8 years ago

I have some questions about the new method in AgendaSkin for the popup.

  1. I don't know how to write the method. Do you want to write it yourself?
  2. Is that approach better than changing the callback argument to Pair<Appointment, Node>?
tbee commented 8 years ago

You need a method to provide you with a node to bind the popup to, so add to AgendaSkin

And then implement that in the Abstract skin.

Yes it is better, because wrapping a Node in a Pair still exposes it through Agenda, only obfuscated.

daviddbal commented 8 years ago

You explained the part that I do know how to do. The implementation is the part I don't know how to do. First, when you say Abstract skin do you mean AgendaSkinTimeScale24HourAbstract? Second, how do I match up Appointments to Nodes. I looked the class over and I can't find a way. Can you please give me more help?

tbee commented 8 years ago

How did you get to a pane for an appointment in your implementation?

daviddbal commented 8 years ago

I used a map. Can I use another map, but keep it in the skin, and access it through the method?

tbee commented 8 years ago

Yup. That would be ok. Make sure it is weak.