Open triksi254 opened 1 month ago
Name | Link |
---|---|
Latest commit | 32e08e901888cda68b54332d10ccfc23f22410ef |
Latest deploy log | https://app.netlify.com/sites/molevolvr/deploys/671d5d76b38dbb00098ae0e2 |
@vincerubinetti @jananiravi please review and give feedback, thank you
A couple of things just briefly looking at the code and not testing it yet:
package-lock.json
should not be there, and you should install things with bun install
instead of npm install
bun run lint
in the /frontend
directory.type
instead of interface
for type declarations.React.FC<NightingaleMSAWrapperProps>
type is fine, but to be consistent with the rest of the code base, it should really just be named Props
. Then you'd do { sequences, features }: Props
. See other components for examples.It's perfectly okay that you've set it up the way you did, because I didn't provide any guidelines in the issue, but I'll want to make a few architectural changes:
mouseOverFillColor
. We want components to have some options for customization to fit our needs, but mostly they should be simple to use from the outside and handle things automatically.Outputs.tsx
section, let's add it to the Testbed.tsx
page with completely fake, generated data, like we're doing with the network visualization component. NewAnalysis.tsx
page which takes whatever the user has inputted and pass it to the MSA component. I'm not sure if we're doing much visualization of raw inputs... that is, I think the data that will ultimately be passed to the MSA component will be coming from the backend, from analysis results, not something that the user inputted verbatim. However I could be very wrong on this. Even if I am wrong though, I will want to set up a generic way that passes all the raw inputs through to the results page, so that any visualizations we have there can use any of the raw inputs. As such, we don't need the changes to NewAnalysis.tsx
yet.I'll probably have some more comments on Monday.
@triksi254 If you're able to make the changes I discussed above, please do. If not, I won't be able to get to making the changes myself until next week.
I am currently working on it. It should be ready tomorrow
On Tue, Oct 15, 2024 at 9:38 PM Vincent Rubinetti @.***> wrote:
@triksi254 https://github.com/triksi254 If you're able to make the changes I discussed above, please do. If not, I won't be able to get to making the changes myself until next week.
— Reply to this email directly, view it on GitHub https://github.com/JRaviLab/molevolvr2.0/pull/32#issuecomment-2414744080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVOAL36Z2HXWVEHJTHDUE5LZ3VOKTAVCNFSM6AAAAABP2UFNPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJUG42DIMBYGA . You are receiving this because you were mentioned.Message ID: @.***>
bun run lint
test shows the error below
bun: File name too long:
error: script "lint" exited with code 1
I'm not able to reproduce that error. Can you provide more info, e.g. which commands you ran, what OS you're using, what other logs (errors or otherwise) did you get?
$ eslint . --fix && prettier **/*.{tsx,ts,module.css,css,html,md,json,yaml} --write --no-error-on-unmatched-pattern
=============
WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
You may find that it works just fine, or you may not.
SUPPORTED TYPESCRIPT VERSIONS: >=4.7.4 <5.6.0
YOUR TYPESCRIPT VERSION: 5.6.2
Please only submit bug reports when using the officially supported version.
=============
bun: File name too long:
error: script "lint" exited with code 1`
This is the full error. Am using windows 10-64 bit
Also, this typescript is defining a custom element for nightingale...
declare namespace JSX {
type IntrinsicElements = {
"nightingale-msa": any;
};
}
...but it is overwriting/removing the definitions for all other native HTML elements:
I believe to capture both it should be:
namespace JSX {
interface IntrinsicElements {
"nightingale-msa": JSX.HTMLAttributes<CustomElement>;
}
}
Thanks, I'll check it out!
On Wed, Oct 16, 2024 at 6:33 PM Vincent Rubinetti @.***> wrote:
Also, this typescript is defining a custom element for nightingale...
declare namespace JSX { type IntrinsicElements = { "nightingale-msa": any; };}
...but it is overwriting/removing the definitions for all other native HTML elements: Screenshot.2024-10-16.at.11.15.21.AM.png (view on web) https://github.com/user-attachments/assets/479bdc62-c8ad-4007-b685-c50449b8cbb4
I believe to capture both it should be:
namespace JSX { interface IntrinsicElements { "nightingale-msa": JSX.HTMLAttributes
; } } — Reply to this email directly, view it on GitHub https://github.com/JRaviLab/molevolvr2.0/pull/32#issuecomment-2417191241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVOAL3YJWF65WRFZXROZIF3Z32BONAVCNFSM6AAAAABP2UFNPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJXGE4TCMRUGE . You are receiving this because you were mentioned.Message ID: @.***>
See what I did in this PR for more guidance:
Step 1: Install Dependencies npm i @nightingale-elements/nightingale-msa
Step 2: Create a Wrapper Component New file NightingaleMSAWrapper.js
Step 3: Integrate the Wrapper Component Modify your Outputs component to include the NightingaleMSAWrapper
Step 4: Parse the MSA Data in NewAnalysis.tsx
Step 5: Integrate the Parsed Data into the Outputs Component