nfdi4plants / Swate

Excel Add-In for annotation of experimental data and computational workflows.
https://swate-alpha.nfdi4plants.org
MIT License
31 stars 6 forks source link

Make annotation column insert dependent on selected column in the table #24

Closed kMutagene closed 3 years ago

kMutagene commented 4 years ago

related SO post

kMutagene commented 3 years ago

Since you can restrict the term search via the column position this should now be trivial @Freymaurer right?

Freymaurer commented 3 years ago

Will look into this!

Freymaurer commented 3 years ago

Well implementing this feature had a way larger overhead then first expected. By inserting new building blocks next to the selected column we pushed hidden columns to the side which could lead to the wrong columns being hidden. Whether a column is hidden or not is determined by the address and will not be pushed to the side together with the column. To circumvent this, i added a hidden tag to the column name. Example:

Parameter [light unit] Term Source REF [light unit; #h] Term Accession Number [light unit; #h]

Everytime the SyncContext message is dispatched the app will unhide all columns and check for this tag to then hide all #htagged columns.

i found an additional problem when one of the previous building blocks was selected. So let's say a cell from Parameter [light unit] from the prev. example is selected. The expected outcome is that the new buildingblock will be created one column to the right. But in fact we want it to be placed AFTER all hidden columns and not directly after the selected col. This is why i needed to implement some kind of logic to compare the requested index with it's following columns and whether or not they have the hidden tag.

All of this was added in commit c3fd679 and will soon be merged to developer.

Tell me if these commit descriptions are to detailed.

kMutagene commented 3 years ago

whelp as long as it works now its not so bad that it was not trivial i guess, nice work😆 Regarding your commit message: Your referenced commit could be more granular, its pretty obvious by the message:

  • Add minor versioning features with a versioned api (just added v1 t… <- commit msg gets cut of here …o api name) and by adding a bottom fixed Swate version to client.
  • AddBuildingBlock now adds the Block to the right side of the selected column (issue #24)

try keeping the description concise enough to not get cut off (i know its sometimes not possible but in this case it is). So the first commit could be

Add minor versioning features with a versioned api

and the second one is then slightly too long as well. You could keep imperative language and do something like

Improve AddBuildingBlock to add new blocks right to selected col

As a rule of thumb, keep your summary line below 72 characters. You can then add a line break to add more info if the commit really need sit, but the 'problem' here is more that you have 2 commits in 1. No need to change that now though, just keep these in mind for your next commits. You can also check the contribution guidelines in BioFSharp. Actually, we might consider adapting them for nfdi4plants repos.