aerie-ui
aerie-ui copied to clipboard
Sequence Editor Followup
The following are some things that that we found from the sequence editor PR that were left out of the review in order to quickly push it through:
### Tasks
- [ ] `DictionaryTable.svelte` - maintaining two different selections based multiselect can potentially become difficult to maintain consistent interactions. Refactor so that potentially only one list of selected IDs is maintained
- [ ] https://github.com/NASA-AMMOS/aerie-ui/issues/1292
- [ ] `ArgumentTooltip.svelte` - Confirm that for displays like `arg.bit_length ?? 'None'` the intent is to show `0` instead of `None` when the value is `0`
- [ ] `ArgTitle.svelte` - title should be set to variable that is set in a reactive statement to prevent unnecessary re-renders
- [ ] Expansion/Parcel/Sequence update queries should be combined into one
- [x] `NumEditor.svelte` - input element should be of type `number` instead of `string`.
- [x] `NumEditor.svelte` - cleanup unused code
- [ ] `SelectedCommand.svelte` - address TODOs
- [x] Repeat arg hovering doesn't trigger a tooltip in editor
- [ ] Remove `utils.ts` as that was renamed to `codemirror-utils.ts`
- [ ] Add unit tests for all the new utility TS files under `/codemirror` and `/new-sequence-editor`
- [ ] `dictionaries/+page.svelte` - the `'Adaptation'` case can be changed to use the enum representation
- [ ] Is it necessary to keep `logger.ts`? Shouldn't those errors/messages be more exposed?
- [ ] Address any TODOs in `/new-sequence-editor` if addressable
- [ ] Move `sequencer-grammar-constants.ts` to a new `/constants` directory for easier access across project
- [ ] `time-utils.ts` - I don't think that the `g` flag is required for each of the regular expressions. Once those are removed, setting the `regex.lastIndex` would no longer be necessary.
- [ ] `time-utils.ts` - For `isTimeBalanced`, rather than passing the regex, can we just pass in defined type constants or enums (i.e. 'ABSOLUTE_TIME') so that the arguments are much more explicit
- [ ] Can `time-utils.ts` be combined with our existing time util file?
- [ ] `grammar.test.ts` - update to be more understandable and less fragile @joswig?
- [ ] Delete `aerie-phoenix-wordmark.svg` if not being used @joswig
- [ ] `SearchableDropdown.svelte` - @joswig I think that displaying a message somewhere on the list (bottom maybe?) would be necessary if the list is truncated to avoid confusion
- [ ] `permissions.ts` - Rename permission for `commandDictionary` to generic `dictionary` if it's no longer command dictionary specific. (`DELETE_COMMAND_DICTIONARY` should be renamed as well if that's no longer relevant)
- [ ] Prevent clicking on download/copy buttons when nothing is selected @goetzrrGit
- [ ] Vertically align the "filter to my sequences" checkbox and text on the sequencing page
- [ ] Style the editor tooltip for better legibility
- [ ] Style the Selected Command section with proper header and Aerie components
- [ ] Discuss why the sequence editor has its own branding
- [ ] Empty state for third column in sequence editor
- [ ] Document usage of `globalThis`
- [ ] Rename "new-sequence-editor" folder to "sequence-editor" in utils
- [ ] De-dupe and comment `isTimeBalanced` which can be found in both `time-utils.ts` and `timeDiagnostics.ts`
- [ ] More docstrings for `time-utils.ts` functions
- [ ] Move the new dependencies from `devDependencies` to `dependencies` in package.json
### Optional Tasks
- [ ] `fswCommandArgDefault` in command-dictionary.ts can be in a switch statement
- [ ] can we change queries like `DELETE_PARCEL_TO_PARAMETER_DICTIONARIES` to be more English friendly like `DELETE_PARCEL_DICTIONARY_ASSOCIATION`?
- [ ] More comments in functions with long and deeply nested conditional statements like `commandLinter`