DigitalCommons / mykomap

A web application for mapping initiatives in the Solidarity Economy
3 stars 0 forks source link

[Dotcoop - Misc] Display # of co-ops filtered in UI #230

Open ColmDC opened 5 months ago

ColmDC commented 5 months ago

Please track against the clockify project [Dotcoop - Misc]

Description

Implement the design chosen in #232 to display the number of results found matching the text search and filter options chosen.

Designs: https://xd.adobe.com/view/aefe2e1a-612a-4ed4-b570-0bf513927e8c-fbad/grid Updated designs 3/4/24: https://xd.adobe.com/view/06927e2d-af74-4e6f-8749-35a116f28e33-53c1/grid

Note the significant additional change here to have the filter results open in a new panel instead of underneath, unifying the design pattern between the directory panel and the filter panel. On mobile only one panel is displayed at a time, so if a user wishes to add more than one filter they will be switched to the new panel unexpectedly which would be a bad UX. Hence the addition of the 'Apply Filters' button to enable the user to add multiple filters on mobile.

Acceptance Criteria

ColmDC commented 3 months ago

Be aware of https://github.com/DigitalCommons/mykomap/issues/238 and https://github.com/DigitalCommons/mykomap/issues/197 while implementing this Issue.

rogup commented 3 months ago

For the dark panel displaying filter results when using the Directory, I can see in the code that there is logic which makes the background different colours, depending on the type of initiatives (housing, energy, etc.). But this seems to be customer-specific, for a specific source of data. See am20, etc in the mykomap style.css file.

@wu-lee or @ColmDC : What does am refer to and is this colour-coding still something that we want to keep? This would complicate things because we need to use the same panel for search results, and the colours wouldn't make sense for a more complicated search. And in any case, I think it's strange that we have hardcoded customer-specific logic in the mykomap repository.

Maybe we could just colour code types of initiative randomly/sequentially to simplify things. But I think this would fall out of the scope of this ticket, and for now we could just use a black panel for everything?

rogup commented 3 months ago

Also, regarding re-using the black panel, I'm wondering if we need the panel to stay exactly the same for the Directory sidebar? (see below)

Screenshot 2024-04-10 at 17 06 36

I think the code could be simplified if the black panel was the same for both the Directory and Search. This would mean that the title at the top of the black panel would be removed (but we could add logic so that the selected type remains highlighted in the leftmost Directory panel). And maybe it would make sense in this case to replace the 'x' close button with 'clear selection' instead of 'clear filter' as suggested in the design? @ms0ur1s @ColmDC do you think we could get away with these small changes, to make the panel's appearance consistent and allow the code to be a lot simpler?

ms0ur1s commented 3 months ago

@ms0ur1s @ColmDC do you think we could get away with these small changes, to make the panel's appearance consistent and allow the code to be a lot simpler?

@rogup, for my part I think that all makes great sense, and I'd go ahead with what you suggested. What's your opinion @ColmDC?

ColmDC commented 3 months ago

It's hard for me to follow exactly the changes you are proposing, but

With dropping the title 'the title at the top of the black panel ', just be conscious that it's not needed in the narrow screen.

But I think it go ahead and implement, using your best judgement.

rogup commented 3 months ago

@ColmDC This is almost ready for testing. A couple of things in the acceptance criteria:

It should be enabled/disabled via map configuration.

Do we definitely need this? It seems like a feature that most users would probably find useful, so could we enable it for everyone? This would keep our config options simpler.

Palette should be configurable with other widget palettes.

What does this mean?

ColmDC commented 3 months ago

Do we definitely need this? It seems like a feature that most users would probably find useful, so could we enable it for everyone? This would keep our config options simpler.

Agreed. Lets make it standard.

ColmDC commented 3 months ago

Palette should be configurable with other widget palettes.

What does this mean?

Whatever colours are required by the design should be configurable.

But I think we just using the sidebarButtonColour for the text colour is all we need to do.

wu-lee commented 3 months ago

Rohit asked:

For the dark panel displaying filter results when using the Directory, I can see in the code that there is logic which makes the background different colours, depending on the type of initiatives (housing, energy, etc.). But this seems to be customer-specific, for a specific source of data. See am20, etc in the mykomap style.css file.

@wu-lee or @ColmDC : What does am refer to and is this colour-coding still something that we want to keep? This would complicate things because we need to use the same panel for search results, and the colours wouldn't make sense for a more complicated search. And in any case, I think it's strange that we have hardcoded customer-specific logic in the mykomap repository.

am is for "Activities Modified" and refers to the linked data vocabulary with the same name.

SEA map, as it was then, was hard-wired to use this vocab for colouring the directory, the marker pins, and possibly other places. This means if anything else is used, the colour used is black. A known problem, but one which hasn't been fully fixed to date, probably just because of time and priorities. I have some ideas for how to fix.

Hunting for relevant issues:

Maybe we could just colour code types of initiative randomly/sequentially to simplify things. But I think this would fall out of the scope of this ticket, and for now we could just use a black panel for everything?

I think it's out of scope. Fixing this needs considering all the various parts: markers, directory, custom styles thereof and the possibility of various vocabs (or none) being used for the initiative's (AKA organisation, marker, pin) attribute in question.

wu-lee commented 3 months ago

Do we definitely need this? It seems like a feature that most users would probably find useful, so could we enable it for everyone? This would keep our config options simpler.

Agreed. Lets make it standard.

Be aware that making it standard means that maps which don't have this now will at some point suddenly start having it, maybe to the surprise of the customer using it. Would they care? I don't know, but they might. I think you're not proposing this would be unconfigurable - if not, we could then turn it off in advance, but it would need to be something we're aware of whenever deploying upgrades. For this reason I'd slightly be in favour of making this backward compatible by default, at least until the next major release.

ColmDC commented 3 months ago

Be aware that making it standard means that maps which don't have this now will at some point suddenly start having it, maybe to the surprise of the customer using it. Would they care? I don't know, but they might. I think you're not proposing this would be unconfigurable - if not, we could then turn it off in advance, but it would need to be something we're aware of whenever deploying upgrades. For this reason I'd slightly be in favour of making this backward compatible by default, at least until the next major release.

At @rogup 's suggestion I was happy for it not to be configurable. I can't imagine it being a problem for anyone. If it is, we could make it configurable then?

rogup commented 3 months ago

@wu-lee I think this relates to what you were saying in the All Hands about standardising Mykomaps so we don't have so many different client-specific versions to develop, test, and maintain. The fewer config options we have, the less things that can go wrong, and the less work there is for us.

So my opinion is to always try to avoid adding config options, unless it's something a client is very likely to not accept. And maybe in these cases, we should have a process where the feature is enabled for everyone and then, if nobody complains about it, the config options is removed altogether in the next release.

wu-lee commented 3 months ago

I'm thinking of the ICA, I get the impression they're quite particular about how the map looks based on their feedback.

@ColmDC - this is more your call than mine.

I confess I've not seen the changes this implements. So this specific case might be arguable. And I agree that making things simpler to deploy is an aim.

But on the general principle "always avoid adding more config options" - it seems untrue to say that a map which is hardwired to one feature set is always a win just because it reduces configuration/implementation effort. A "reductio ad adsurdum" argument seems to be possible here.

Using the same principle of "simpler is better" could be used to argue that a better solution than not implementing the config would be not implementing this entire feature. But the reason we are is that it was asked for by DotCoop - for their specific use-case.

We can't always anticipate what clients will accept, and they've all got their own quirky and subjective opinions. Forcing an arbitrary feature which client A asked for onto client B, C, and D etc. seems like a risky proposition.

WRT to reducing complexity and increasing our agility delivering maps: a major source of our difficulties in managing variations between maps comes from the need to deal with different data sets, with different fields, which need to be handled differently. And showing this in the pop-up, and all the styling on top of that. We can't really get rid of that configuration - and that sort of configuration totally dwarfs the above sort of configuration. The need to do this configuration is the primary reason why, as Lynne put it "Creating a new map requires a bunch of meetings and a decent amount of dev".

In terms of standardisation, what I was suggesting is to tackling that aspect. And probably at the application level (i.e. build a "Mykomap project to end all Mykomap projects" - for a defined set of cases which cover what we need.)

Sorry for being so verbose.

ColmDC commented 3 months ago

To close this out, let's have it NOT configurable. It is not one of our complicated configuration issues.

ColmDC commented 2 months ago

What's blocking this moving into QA?

ColmDC commented 2 months ago

Does the Clear Filter button now reset the filters to the startup configurations or reset them all to empty (text box) and -Any- (filters)?

rogup commented 2 months ago

@ColmDC 'Clear filters' resets all the filters to -Any- and empties search box. What do you mean by the startup configurations?

ColmDC commented 2 months ago

Clear filters' resets all the filters to -Any-

Good. That is what is expected. I just noticed that since the ability to configure filters to be set on map startup, Clear filters resets the filters to the startup configuration, and doesn't clear them. See https://www.workers.coop/map/.

rogup commented 2 months ago

@ColmDC This is now ready for QA, available here https://dev.maps.solidarityeconomy.coop/qa

ColmDC commented 2 months ago

Cool. @ms0ur1s could you take a look and see if it behaves as you expect?

ms0ur1s commented 2 months ago

Cool. @ms0ur1s could you take a look and see if it behaves as you expect?

No worries @ColmDC, will give a once over now.

ms0ur1s commented 2 months ago

@ColmDC and @rogup all looking good.

Only problem I noticed was on the mobile view. The close button is no longer working on the info panel for a selected, filtered result. So that once the panel is opened there is no way of navigating back to the search results other than by refreshing.

Steps to recreate:

Tested using dev tools, my iphone 6s and across multiple devices using an in-browser mobile simulator extension (Mobile FIRST - definitely not infallible, but generally dependable).


image

ColmDC commented 2 months ago

I see that too.

What did we decide about the Apply Filters button? Were we going to wait until it was clicked before applying filters and search text matching? Were we going to do it in Mobile and Large screen? It seems the button disappears in Large screen mode?

I'm also seeing with the ICA example, the countries are not displayed when you first run it up. They appear if you click Directory icon. (The bug is not exibited in DotCoop map.)

@rogup shall I write these up as fresh issues?

ms0ur1s commented 2 months ago

What did we decide about the Apply Filters button? Were we going to wait until it was clicked before applying filters and search text matching? Were we going to do it in Mobile and Large screen? It seems the button disappears in Large screen mode?

I think this functionality is fine, as the Apply Filters button is unnecessary on large screens. It all reduces the amount of actions necessary before results are returned.

rogup commented 2 months ago

I think the Apply Filters button behaviour matches the spec that was decided.

I'm happy to fix those 2 regressions within this issue @ColmDC since they were caused by these code changes. Unless you would prefer to make new issues. The regressions are:

  1. The close button not working on the initiative info sidebar when in mobile mode
  2. The individual directory entries not appearing when some maps (e.g. ICA) are first loaded, probably due to a race condition
ColmDC commented 2 months ago

Please do see if these regressions are quick to fix. If not then let's create issues for them.

ColmDC commented 2 months ago

I think at some point we had decided that the apply filters button would be on all screen sizes and that the filtering would not be triggered until the button was pressed, but I can see that this didn't make it across into this ticket. Would it be straight forward to make those changes?

ColmDC commented 2 months ago

Let's just fix the regressions. Add furhter chnages would require new issue.

ColmDC commented 1 month ago

Just checking that I'm not a block on this one.

rogup commented 1 month ago

No, I've been in the middle of other work so haven't got to this yet, but planning to finish it this week.

rogup commented 1 month ago

@ColmDC The regressions are now fixed and the new maps are deployed to https://dev.maps.solidarityeconomy.coop/qa for testing

ColmDC commented 1 month ago

Looking good. Behaves well on mobile now too. :-)