-
Notifications
You must be signed in to change notification settings - Fork 1
Added subtitle, removed editable title #105
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
Conversation
Summary of ChangesHello @kevindetry-milaboratories, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the display of block titles by transitioning from an editable field to a static 'Samples & Data' title. To enhance user context, a new dynamic subtitle has been implemented, which provides real-time information about the number of datasets. Additionally, this PR includes comprehensive updates to various project dependencies, ensuring the use of the latest library versions and improving overall stability and compatibility. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request removes the editable block title, replacing it with a static title and a new dynamic subtitle that displays the number of datasets. The changes are consistent across the model, UI, and workflow, and the implementation is straightforward. My main feedback is about the repeated use of the hardcoded string "Samples & Data" in multiple files. To improve maintainability and adhere to the DRY principle, this string should be defined as a shared constant.
| ) | ||
|
|
||
| .title((ctx) => ctx.args.blockTitle ?? 'Samples & Data') | ||
| .title(() => 'Samples & Data') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title 'Samples & Data' is hardcoded. This string is also used in other places, such as ui/src/pages/MetadataPage.vue and workflow/src/main.tpl.tengo. To follow the DRY (Don't Repeat Yourself) principle and improve maintainability, consider defining this as a constant in a shared location (e.g., in the model package) and importing it where needed. This makes future changes to the title easier to manage across the application.
| placeholder="Samples & Data" | ||
| /> | ||
| </template> | ||
| <PlBlockPage title="Samples & Data"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| trace := pSpec.makeTrace(undefined, | ||
| {type: "milaboratories.samples-and-data", id: blockId, importance: 10, label: blockTitle}, | ||
| {type: "milaboratories.samples-and-data", id: blockId, importance: 10, label: "Samples & Data"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label 'Samples & Data' is hardcoded. This string is also used in the frontend (model and ui packages). For consistency, it would be ideal to use a shared constant. If sharing a constant between TypeScript and Tengo is not straightforward, at least consider defining it as a local constant at the top of this file to avoid magic strings.
ea2820f to
47e5eaf
Compare
47e5eaf to
ed17db5
Compare
No description provided.