Skip to content

Show whether changes to a method are saved or not#2611

Merged
robertbrignull merged 5 commits intomainfrom
robertbrignull/data-unsaved-checkbox
Jul 18, 2023
Merged

Show whether changes to a method are saved or not#2611
robertbrignull merged 5 commits intomainfrom
robertbrignull/data-unsaved-checkbox

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

This PR replaces the checkbox on each method row with an icon that shows whether that method is modelled, whether there have been any changes to that row, and whether those changes are saved or not.

Screenshot:
Screenshot 2023-07-14 at 16 18 07

Implementing this involved changing how we track changes, to track individual methods instead of entire models. Thankfully this was fairly easy to implement and I think actually made things a little clearer overall. I haven't tried to remember what the original state was, so if you make a change and then revert it then the method will still show as changed and unsaved, but I think this behaviour is acceptable here.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team as a code owner July 14, 2023 15:21
!externalApiUsage.supported ||
(modeledMethod && modeledMethod?.type !== "none");
(modeledMethod && modeledMethod?.type !== "none") ||
modifiedSignatures.has(externalApiUsage.signature);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is attempting to mitigate a but where when you change a method from "modeled" to "unmodeled" and then save it'll temporarily show as "modeled by codeql or another pack". This bug appears on main as well and is in fact more noticeable there. I'm going to look into this and I'm hoping to address it separately.

Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, just some comments about the organization of the code

Comment thread extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx Outdated
Comment thread extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx Outdated
@robertbrignull robertbrignull enabled auto-merge July 18, 2023 15:10
@robertbrignull robertbrignull merged commit d3a5a5e into main Jul 18, 2023
@robertbrignull robertbrignull deleted the robertbrignull/data-unsaved-checkbox branch July 18, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants