Skip to content

SC-180 Added Drawing Button #474

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

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open

Conversation

collins-self
Copy link
Contributor

Added the button to look for a location's drawings within the Asset Connection page.
Was already on the NMW.

@collins-self collins-self force-pushed the SC-180-Drawing-Button branch from f3d0900 to 309bfda Compare August 9, 2024 16:57
@collins-self collins-self changed the title SC-181 Added Drawing Button SC-180 Added Drawing Button Aug 9, 2024
@elwills
Copy link
Member

elwills commented Aug 9, 2024

I don't see it on the Asset Connections page in NMW. Also placement of the button is no good. Also this doesn't work if the Asset has more than one Substation.

@collins-self collins-self force-pushed the SC-180-Drawing-Button branch 2 times, most recently from 3c968f6 to ab53990 Compare August 19, 2024 18:11
@clackner-gpa clackner-gpa force-pushed the SC-180-Drawing-Button branch from ab53990 to ebf210d Compare October 2, 2024 19:05
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch 2 times, most recently from 5f590e8 to c037861 Compare October 7, 2024 18:14
@clackner-gpa clackner-gpa force-pushed the SC-180-Drawing-Button branch from c037861 to 52f0c10 Compare October 8, 2024 16:13
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch from a8bcd7b to dcd5ad4 Compare October 9, 2024 18:35
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch from 183a6ab to 0c8ec5e Compare October 21, 2024 17:48
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch 2 times, most recently from 94d3b84 to 2e5d085 Compare November 4, 2024 20:39
@clackner-gpa clackner-gpa self-requested a review January 7, 2025 14:02
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch 2 times, most recently from 24e3ef6 to e9f2a13 Compare January 14, 2025 12:09
const isValid = (location: OpenXDA.Types.Location, drawingData) => {
let e = "";

if (!location
Copy link
Contributor

@gcsantos-gpa gcsantos-gpa Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!location
if (location != null

We should be explicit about our null checks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should be if (location != null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch, suggestion fixed

Copy link
Member

@clackner-gpa clackner-gpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all comments above are adressed

@collins-self collins-self force-pushed the SC-180-Drawing-Button branch from 38aaa44 to 552ccf8 Compare January 24, 2025 15:34
@collins-self collins-self force-pushed the SC-180-Drawing-Button branch from 82505cc to 2a4628c Compare January 27, 2025 13:51
Copy link
Contributor

@gcsantos-gpa gcsantos-gpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New files/removed files also aren't reflected in the project file. Can you perform the necessary changes to the project file?

Copy link
Contributor

@gcsantos-gpa gcsantos-gpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase threw away some changes made by other PR's

function handleSelect(item) {
history.push({ pathname: homePath + 'index.cshtml', search: '?name=Asset&AssetID=' + item.row.AssetID})
history.push({ pathname: homePath + 'index.cshtml', search: '?name=Asset&AssetID=' + item.row.AssetID })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gcsantos-gpa I bet this conflicts with #606 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would yes

@clackner-gpa
Copy link
Member

@gcsantos-gpa Do not merge please.
I want to give this a shot in a rewrite to simplify significantly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants