[DotCoop – Misc] Support the configuration of which tab is selected on map startup.

[ColmDC]

Please track against the clockify project [DotCoop - Misc]

Description

Support ability to configure a MM to specify what tab is selected on startup within the side panel

Acceptance Criteria

• If no configuration setting present then default to Directory
• Demonstrate it working on DotCoop where the Search panel is visible on startup.
• Demonstrate it working on ICA map with no config settings provided, so it should still default to Directory.
• Demonstrate that it doesn't crash if it is set to a Datasets tab, when that tab is set to invisible.

[ColmDC]

Might be worth addressing while fixing #226

[rogup]

@wu-lee Please could you review my code for this? It's in https://github.com/DigitalCommons/mykomap/tree/231-default-sidebar-tab

I've tested locally the things in the acceptance criteria by checking out https://github.com/DigitalCommons/owned-by-oxford-project/tree/public/dev in /ext

To do those actual tests for DotCoop and ICA, do I just need to do the same thing but with those respective repos checked out in /ext? Or is there another way to test changes non-locally in some sort of staging setup?

Once the feature is ready to be applied, I'd also like to have a call about your process of rebasing the branch.

[rogup]

Also I've noticed that some of the line breaks have changed due to my Prettier code formatter.

Since more of us are going to be working on this, I'd be in favour of all of us using Prettier with the same default config, so that styling doesn't become inconsistent. This might cause a lot of changes for a short while, which makes looking at diffs a bit annoying, but I think it will be worth it in the long run - what do you think?

[rogup]

Oh, and should I be adding a unit test (or any other type of test) for this? If so, where should it go?

And are there any docs yet to describe our testing setup, and how we should be adding tests? I couldn't find any

[wu-lee]

To do those actual tests for DotCoop and ICA, do I just need to do the same thing but with those respective repos checked out in /ext? Or is there another way to test changes non-locally in some sort of staging setup?

Yes, that's how I do it. Although: I check out the map projects in directories adjacent to the mykomap working directory, and use a symlink. I think you're on OS X so this solution should work for you.

On Windows, for various reasons on top of this one, I'd recommend developing using Windows System for Linux. This isn't a commonly used platform for development, even then.

Once the feature is ready to be applied, I'd also like to have a call about your process of rebasing the branch.

Yep.

[wu-lee]

Also I've noticed that some of the line breaks have changed due to my Prettier code formatter.

Since more of us are going to be working on this, I'd be in favour of all of us using Prettier with the same default config, so that styling doesn't become inconsistent. This might cause a lot of changes for a short while, which makes looking at diffs a bit annoying, but I think it will be worth it in the long run - what do you think?

I'm not absolutely against this idea in theory, but - yikes! - I am baulking a bit at reformatting the entire project (and presumably all the other projects?) to support Prettier, which isn't something new to me and to all DCC development to date (i.e. not just me and not just this project). And especially doing that as part of this issue. (I'm not saying you're suggesting that, BTW, but it does seem to be a logical consequence if we did adopt it.)

I don't use VS Code for development, I use Emacs - so I'm wondering if I even can use Prettier. Investigating... yes, it's a stand-alone NodeJS tool, and it does seem to have support for Emacs, which is a relief, as otherwise it might be a step backwards for me.

Next, I was wondering how hard it would be to just tell Prettier to follow the current style in the repo.... (Which isn't totally consistent, but that's partly because of the chequered history, and my slight reluctance to make big whitespace changes like this unless I really have to.)

...However, reading the Prettier docs it says:

Prettier is not a kitchen-sink code formatter that attempts to print your code in any way you wish. It is opinionated. [...] By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles.

So ok: it it sounds like that's not an option. In fact that sounds a bit like Douglas Crockford's JS linter, which when I did try using, found unpleasant and inconvenient in practice exactly because if it's "opinionated and inflexible by design" design. It made me jump through hoops I didn't really feel were very helpful, and typically at times I didn't want to have to tell it to shut up and go away, and ultimately I concluded the simplest things was just to uninstall the little ****er.

So finally, I am wondering if this claim that our lives would be better with Prettier is actually valid.

But, even conceding that Prettier's vision that it adds productivity by silencing sources of dissent in a sane way to apply across the board on new projects... this would still imposes a great big multi-repo alteration which could scupper git-blame's ease of application from there on. Being able to use git-blame is itself a very useful tool for extracting information from the commit history. So I'm not yet convinced that vision offers a compelling benefit over using a formatter which can adopt a formatting style which matches, or closely matches, the one in the code already?

[wu-lee]

Oh, and should I be adding a unit test (or any other type of test) for this? If so, where should it go?

Tests would go in the test/ directory. They're all added by me late in the life of Mykomap, which wasn't designed to be test-first, and so are a bit piecemeal and best-effort. (And currently not all passing, I notice.)

Thing is, I'm not sure if I found a way to test the config very simply. test-model-config.js is an attempt. You can run that by itself with

npm run mocha-test test-model-config.js

Open to ideas.

And are there any docs yet to describe our testing setup, and how we should be adding tests? I couldn't find any

No, the testing that's here is a bit tentative, and added in a period when I was the only developer. I should write something.

[ColmDC]

But, even conceding that Prettier's vision that it adds productivity by silencing sources of dissent in a sane way to apply across the board on new projects... this would still imposes a great big multi-repo alteration which could scupper git-blame's ease of application from there on. Being able to use git-blame is itself a very useful tool for extracting information from the commit history. So I'm not yet convinced that vision offers a compelling benefit over using a formatter which can adopt a formatting style which matches, or closely matches, the one in the code already?

Can we move this discussion to element?

[ColmDC]

No, the testing that's here is a bit tentative, and added in a period when I was the only developer. I should write something.

Ditto.

[wu-lee]

(BTW I'm fine if you want to make this a pull-request - although it is essentially one commit ATM, it gives me somewhere to put the review comments.)

On a basic scan with "ignore whitespace" enabled, which removes some of the Prettier churn, it still a bit hard to find the bits which are not churn, because of all the quote-switching and line-rewrapping.

• Can we revert the Prettier churn or at least put in a separate commit on dev?
• 751c34a: an observation - this fix for the mykomap-site-build wrapper likewise probably deserves to go on the dev branch, as it's not really related to this issue. Can do that later.
• An observation - I note some doc fixes in CONFIG.md due to me fixing the code earlier but not re-generating CONFIG.md.
• I think the changes in this branch add a new string config attribute defaultPanel - in addition to defaultOpenSidebar which sets it open in the first place. Ok. Although:
  • a) That string isn't constrained by anything, I'm thinking perhaps it should be constrained to the valid identifiers for the panels? We can do this without runtime checks in Typescript. (Haven't checked if this is not possible because of the JS legacy design.)
  • b) In fact, maybe we could roll these two options into one, by allowing defaultOpenSidebar to be true or false (for backcompatibility - works as before) or a string which is one of the valid panel identifiers. The only drawback I can think of is that (unless we try to more-cleverer) means we can't have a default panel if it's not opened by default, but I'm not sure if that is something we want to be able to do.

Note, this is all just from eyeballing - I've not yet had time to test this builds and does what it says, will defer that until tomorrow.

[rogup]

@wu-lee I've made a PR and addressed those comments #236

• I've reverted Prettier churn and disabled Prettier for the codebase for now
• I've added 751c34a to dev separately
• I didn't realise CONFIG.md was auto-generated. I've fixed the ts file and then generated them.
• • a) We don't seem to have a hardcoded list of sidebar panels that we can use. sidebar.ts seems to load a list of presenters one after another at runtime. I could define a new type for these panel labels if you think it's worth it? I suppose it would help to catch errors if we ever want to add another type of panel, and we could remove the existing runtime check.
  • b) I thought about using the same config option for both defaultOpenSidebar and defaultPanel and Colm suggested, but decided against it. I think mixing boolean and strings for one variable adds unneeded complexity, both to understand it and in the code. We also get a bit more functionality with 2 separate options, since defaultPanel still works without defaultOpenSidebar enabled, so the chosen panel appears when the user manually opens the sidebar for the first time. So unless there's a big reason why fewer config options is much more preferable, I think separate is better in this case.

[ColmDC]

Where can I test this feature? @rogup

[rogup]

@ColmDC what's the process for making features available for testing, and are there docs for this? I can add them if so

The way I tested it was to checkout 231-default-sidebar-tab and build/run it locally

[ColmDC]

@wu-lee might need to assist here. But from my perspective I need to be able to evaluate it on

https://dev.maps.coop/ica/

Demonstrate it working on ICA map with no config settings provided, so it should still default to Directory.

https://dev.maps.coop/dotcoop/

Demonstrate it working on DotCoop where the Search panel is visible on startup.

[ColmDC]

But if we want to move to alingning our processes, @lin-d-hop might prefer a different approach, like bundling a few more features before creating a new release?

[wu-lee]

What I used to do was to use hook-runner to deploy mykomap in "test mode" on https://dev.maps.coop/temp/

Currently there are hook-runner targets for testing Mykomap with the Co-ops UK map (config: coopsuk_testing.json) and the ObO map (config obo-public_testing.json). These are configured to deploy here:

• https://dev.maps.solidarityeconomy.coop/temp/coopsuk/
• https://dev.maps.solidarityeconomy.coop/temp/obo-public/

The use of temp in the URL is meant to imply "don't rely on these working or even being here!"

What these targets do is redeploy the testing branch of the Mykomap repo in development mode, using the respective map project, and are triggered via webhooks from GitHub on commits to that branch.

To do the ICA would need adding a new hook-runner target which tests Mykomap using that map project. Alternatively you could add the option you're testing to one of the existing map projects (maybe ObO?) It'd need to be on the public/dev branch for the owned-by-oxford-project map.

And then you reset the testing branch of the Mykomap repo to be on the commit you want to test. Then when you push this branch to GitHub, it triggers a rebuild, which can be tested on the respective URL above. (The testing branch is intended for being yanked around like this, although with two of us we'd need to coordinate.)

Hope that makes sense! If not I can try and explain better.

[wu-lee]

Note, this is all done on dev-1, which can't use the dev.maps.coop as that's pointed at dev-2. When we migrate dev-1 properly the dev.maps.solidarityeconomy.coop domain will be deleted.

[ColmDC]

Doesn't seem to make any sense to start doing more on dev-1.
Seems like we need do whatever @rogup needs to be able to deploy to dev-2 so I can test dev.maps.coop?

[rogup]

Yeah maybe it doesn't make sense to add new hookrunner targets for ICA and DotCoop to dev-1 if things are moving to dev-2.

I'm guessing adding hookrunner targets to dev-1 is less work than setting up the whole hookrunner on dev-2? But maybe we should bite the bullet and do it since we'll need this soon anyway.

I think @wu-lee is best placed to set up the hook runner on dev-2 since he made it and knows how it works for mykomap. Then I'm happy to sort out the testing branch to point at this fix.

@ColmDC up to you to decide how to prioritise this, so that the testing of this feature can be unblocked.

[wu-lee]

The only problem with putting things like this on dev-2 is that there's a bunch of stuff set up on dev-1 already. So it means either having two hook-runners running in parallel, which is confusing in various ways, or moving the whole lot, which is much more work and adds more cans of worms to something which is otherwise relatively simple. (All the git projects need reconfiguring manually to send their webhook notifications to a new URL, for example.)

Just adding this to the existing dev-1 instance would mean that it gets moved with everything else when that happens - so it won't get lost.

If we want to do this quick, I recommend using the hook-runner we have now.

The move to dev-2 can be done too, perhaps in parallel, but seems to be lower down the priority than various other tickets, including this one, and so this issue might get blocked by that.

[rogup]

@colm This is ready for QA.

I've set up the hook runner according to the Git workflow designed here by me and @wu-lee , for the ICA and DotCoop maps.

Since this feature was merged into dev, the QA builds were triggered and are available at https://dev.maps.solidarityeconomy.coop/qa/

Note that these are still on dev-1. It was too much work to move everything to dev-2 at this stage, since the hook runner isn't set up there, but once we have trialled this on dev-1 and setup some automation scripts, it should be easier to move it over to dev-2.

[ColmDC]

Demonstrate it working on DotCoop where the Search panel is visible on startup.
For the full effect I realise defaultOpenSidebar also needs to be set to true which annoyingly doesn't seem to be settible by url,
but the feature as spec'ed works.

Demonstrate it working on ICA map with no config settings provided, so it should still default to Directory.
As above.

Happy to close this, and can someone set defaultOpenSidebar to be true for future dotcoop releases.

[wu-lee]

Happy to close this, and can someone set defaultOpenSidebar to be true for future dotcoop releases.

I've updated this on the dev branch and pushed - this has updated the deployed version here:

https://dev.maps.solidarityeconomy.coop/qa/dotcoop/

The version here used to be deployed by pushes to dev but I think has to be manually deployed with a push to staging now, is that right @rogup? (BTW your diagram link above goes to matrix.to?)

https://dev.dotcoop.solidarityeconomy.coop/

[rogup]

Here's the link to the editable diagram: https://drive.google.com/file/d/1Fu8zt5fdMXIXnyZz944CqwAo-vKdsFYY/view?usp=sharing

https://dev.dotcoop.solidarityeconomy.coop/ now needs to be manually deployed, by setting up the alpha branch on the dotcoop repo, and then clicking the dotcoop_alpha target on the hook runner. There are more details instructions on the diagram, which maybe we could eventually move to the Mykomap Wiki or docs

[wu-lee]

This has been tested - seems to be working, and is ready for merging to the main branch.