-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Dotcoop - Misc] Display # of co-ops filtered in UI #230
Comments
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 @wu-lee or @ColmDC : What does 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? |
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) 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? |
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. |
@ColmDC This is almost ready for testing. A couple of things in the acceptance criteria:
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.
What does this mean? |
Agreed. Lets make it standard. |
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. |
Rohit asked:
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:
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. |
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? |
@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. 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. |
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. |
To close this out, let's have it NOT configurable. It is not one of our complicated configuration issues. |
What's blocking this moving into QA? |
Does the Clear Filter button now reset the filters to the startup configurations or reset them all to empty (text box) and -Any- (filters)? |
@ColmDC 'Clear filters' resets all the filters to -Any- and empties search box. What do you mean by the startup configurations? |
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/. |
@ColmDC This is now ready for QA, available here https://dev.maps.solidarityeconomy.coop/qa |
Cool. @ms0ur1s could you take a look and see if it behaves as you expect? |
@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). |
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? |
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. |
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:
|
Please do see if these regressions are quick to fix. If not then let's create issues for them. |
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? |
Let's just fix the regressions. Add furhter chnages would require new issue. |
Just checking that I'm not a block on this one. |
No, I've been in the middle of other work so haven't got to this yet, but planning to finish it this week. |
@ColmDC The regressions are now fixed and the new maps are deployed to https://dev.maps.solidarityeconomy.coop/qa for testing |
Looking good. Behaves well on mobile now too. :-) |
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
The text was updated successfully, but these errors were encountered: