Skip to content

Commit

Permalink
Fixed copy image not displaying base lines
Browse files Browse the repository at this point in the history
  • Loading branch information
@rayzhuca
rayzhuca committed Nov 16, 2023
1 parent 34ff35c commit e438f97
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/src/middleware/export_svg.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ export_svg_middleware(Store<AppState> store, dynamic action, NextDispatcher next
_export_from_element(elt, 'side');
}
} else if (action is actions.CopySelectedStandsToClipboardImage) {
List<Element> selected_elts = get_selected_strands(store);
List<Element> selected_elts = (!app.state.ui_state.show_base_pair_lines

This comment has been minimized.

Sign in to view

Copy link
@dave-doty

dave-doty Nov 16, 2023 •
edited
Loading

Member

A couple of pedantic requests based on the aesthetics that will help me to understand this code better down the road:

  1. I'll occasionally do clever one-liners, but in general I prefer to break things up a bit over a few statements. There's quite a bit going on in this one command starting on line 67 (2 different functions getting called, plus the addAll). Things like this are easier to debug when separated into a few commands.
  2. It's a bit odd to see that get_selected_base_pairs and get_selected_strands each take a Store as input. This might just be pure aesthetics, but somehow it seems more natural to me to pass in a more specific object (e.g., for get_selected_strands, all you access in it is store.state.ui_state.selectables_store.selected_strands, so it makes more sense to me to pass that in as the argument). Also the name seems a bit inaccurate. You already have the selected Strand objects in store.state.ui_state.selectables_store.selected_strands, so that's not really what the function is returning. It's actually returning the HTML nodes that represent the selected Strand objects. A better name for the function would be something like get_svg_elements_of_strands. Note we also remove "selected" from the name, because there's nothing about the function that requires the strands be selected. It could be all strands, or the one that was just clicked, or something, and we could reuse this function later in those other contexts where we want to get the HTML nodes corresponding to some set of Strand's whether or not that set happens to be the Strand's that are currently selected. Similar ideas probably apply to get_selected_base_pairs although I don't have time right now to look at it in more detail.
? []
: get_selected_base_pairs(store))
..addAll(get_selected_strands(store));
if (selected_elts.length != 0) {
_copy_from_elements(selected_elts);
}
Expand Down

0 comments on commit e438f97

Please sign in to comment.