Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add board type information to mfmc file #1530

Merged
merged 4 commits into from Dec 26, 2023
Merged

Add board type information to mfmc file #1530

merged 4 commits into from Dec 26, 2023

Conversation

DocMoebiuz
Copy link
Collaborator

@DocMoebiuz DocMoebiuz commented Dec 25, 2023 •
edited

fixes #1521

  • On opening - Message popping up to inform when no board type is contained in file.
    image
  • On opening - Message popping up to inform that there is a board type mismatch.
    image
  • On opening - In both cases user can continue with "Yes" or stop opening with "No"
  • On saving - board type is saved as new property
  • Documentation - Add info to https://github.com/MobiFlight/MobiFlight-Connector/wiki/Mobiflight-Connector-Main-Window#mobiflight-modules---tab
  • i18n
  • unit tests
    • extract methods for open and save logic
    • add unit tests for open and save
    • add missing equals implementations in various classes
    • added ToString implementations in various classes
    • refactored ToString method to new style
  • remove all unnecessary usings in config namespace

@github-actions GitHub Actions
Copy link

Build for this pull request:
MobiFlightConnector.zip

@github-actions GitHub Actions
Copy link

Build for this pull request:
MobiFlightConnector.zip

@github-actions GitHub Actions
Copy link

Build for this pull request:
MobiFlightConnector.zip

neilenns
neilenns reviewed Dec 26, 2023
View reviewed changes
UI/Panels/Settings/MobiFlightPanel.cs Outdated
var message = string.Format(i18n._tr("uiMessageOpenConfigUnspecificTypeText"), module.Type);

// New configs have a specific type other than ""
if ((newConfig?.ModuleType ?? "") != "")
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit of a style thing, but I prefer using String.IsNullOrEmpty() when testing strings for empty values instead of "".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point. they introduced that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It reads better, yet I don't understand why some of these are not simply an extension method.

neilenns
neilenns suggested changes Dec 26, 2023
View reviewed changes
Copy link
Contributor

@neilenns neilenns left a comment

Choose a reason for hiding this comment

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

Will comment more in discord

@DocMoebiuz
Copy link
Collaborator Author

image

What's a better wording?

@neilenns
Copy link
Contributor

image What's a better wording?

Maybe

This file does not have a board type associated with it and you are trying to load it on a {board type}. It may be incompatible, are you sure you want to do this?

@DocMoebiuz
Copy link
Collaborator Author

The file doesn't contain any board type information. It might be incompatible. Your board type is "....".

I think ideally we could tell the user that it will be saved next time. Just so that they know how to get rid of this message.

@DocMoebiuz
Copy link
Collaborator Author

Save the file again to associate it with the current board type.

@github-actions GitHub Actions
Copy link

Build for this pull request:
MobiFlightConnector.zip

@DocMoebiuz DocMoebiuz merged commit f293dd4 into main Dec 26, 2023
1 check passed
@DocMoebiuz DocMoebiuz added the enhancement Improvement of an existing feature label Dec 26, 2023
@DocMoebiuz DocMoebiuz mentioned this pull request Dec 31, 2023
Closed
@DocMoebiuz DocMoebiuz added feature A new feature and removed enhancement Improvement of an existing feature labels Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neilenns neilenns neilenns requested changes

Assignees
No one assigned
Labels
feature A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mfmc files contain board type
2 participants
@DocMoebiuz @neilenns