bmlt-enabled / BMLTSearch3

Android and IOS meeting search app
https://bmltsearch.bmlt.app/
0 stars 4 forks source link

Only show virtual meeting info/link for virtual meetings #28

Closed tempsaint closed 2 years ago

tempsaint commented 2 years ago

Currently, meeting info shows the physical location and address for meetings that are "Virtual (Temporarily replacing in-person)." My suggestion is the only have the button for the link to join the meeting and the virtual meeting additional info in the meeting information box.

For In-Person meetings it would show the physical location and address and have the "Directions" button. For Virtual or Virtual (temporarily replacing an in-person) meetings it would NOT show the information in the Location and Address fields and only show the virtual meeting additional information while having buttons for "Virtual Link" and "Phone Meeting Dial-in" For Hybrid (both in-person and virtual) it would show the Location, Address, and Virtual Meeting Additional Info as well as have all three buttons (Directions, Virtual Link, Phone Meeting Dial-in).

paulnagle commented 2 years ago

Sure, this makes sense. I'll add this to the next release.

paulnagle commented 2 years ago

Fixed in #30

tempsaint commented 2 years ago

Paul, I know you closed this and said you would implement it but the newest version doesn't do this. Here's a screenshot I just took this morning. This is a virtual meeting temporarily replacing an in-person one and the physical location info is still being displayed. Screenshot_20220130-092620

pjaudiomv commented 2 years ago

I think this is the logic i use, if helpful. I have it in every language but js or ts lol https://github.com/bmlt-enabled/meetings-by-venue-type/tree/main/src

https://github.com/bmlt-enabled/meetings-by-venue-type/blob/main/src/php/main.php#L104

paulnagle commented 2 years ago

OK, so I think the format that was catching me out was "Virtual (temporarily replacing an in-person)". It looks like there is not a separate shortcode for this format, but a meeting is a "Virtual (temporarily replacing an in-person)" meeting if it has both the TC (temporarily closed) format and Virtual Meeting formats. Is that about right?

tempsaint commented 2 years ago

Ah, yes. I didn't take a look at your logic yet, but basically, if the meeting is VM, then the location, address, and extra info fields should be ignored because those all pertain to physical location. For virtual only meetings those fields should be empty and for virtual meetings that are temporarily replacing an in-person, there's info in the fields but we want to ignore it because it's not pertinent while the meeting is virtual.

But if you want to specifically query for VM & TC then that works.

tempsaint commented 2 years ago

I'm looking at the php code specifically... this is the section we're talking about that makes the determination which function to use to display the different info?

foreach ($meetings as $meeting) {
    $formats = explode(",", $meeting['formats']);
    if (!in_array("VM", $formats) && !in_array("TC", $formats) && !in_array("HY", $formats)) {
        $inperson++;
    } elseif (in_array("VM", $formats) && !in_array("TC", $formats) && !in_array("HY", $formats)) {
        $virtual++;
    } elseif (in_array("VM", $formats) && in_array("TC", $formats) && !in_array("HY", $formats)) {
        $tempvirtual++;
        $virtual++;
    } elseif (!in_array("VM", $formats) && !in_array("TC", $formats) && in_array("HY", $formats)) {
        $hybrid++;
    } elseif (in_array("VM", $formats) && !in_array("TC", $formats) && in_array("HY", $formats)) {
        $hybrid++;
    }
    $total_meetings++;
}
paulnagle commented 2 years ago

@tempsaint I've made the changes you requested, and I have them running on https://bmltsearch.bmlt.app to test.

Could you have a look and see if the meeting lists are behaving as you would expect? Thanks

paulnagle commented 2 years ago

Released in https://github.com/bmlt-enabled/BMLTSearch3/releases/tag/v4.01.00