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.

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

🔀🎉 nameSchema module as replacement for v3 renameFz #2872

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from

Conversation

buffcode
Copy link
Contributor

@buffcode buffcode commented Feb 4, 2024

What kind of update does this PR provide? Please check

  • new translations / updated translations / translation fixes
  • a new feature
  • a new module
  • a bugfix for an existing feature / module
  • improvement of style or user experience
  • other: Please fill out

What is included in your update?

  • new module nameSchema
    • rename vehicles at dispatch, building and vehicle level
    • rename building at dispatch and building level
    • multiple strategies and formats for unit/building numbering
    • batch renaming
  • removal of renameFz module

Additional notes

@Cr4zyc4k3
Copy link
Collaborator

temptating = templating? 😄

src/modules/nameSchema/assets/helper/template.ts Outdated Show resolved Hide resolved
src/modules/nameSchema/assets/helper/template.ts Outdated Show resolved Hide resolved
src/modules/nameSchema/assets/helper/template.ts Outdated Show resolved Hide resolved
src/modules/nameSchema/assets/helper/types.ts Outdated Show resolved Hide resolved
src/modules/nameSchema/docs/de_DE.md Show resolved Hide resolved
src/modules/nameSchema/docs/de_DE.md Outdated Show resolved Hide resolved
src/modules/nameSchema/docs/de_DE.md Outdated Show resolved Hide resolved
@Cr4zyc4k3
Copy link
Collaborator

Despite my argument with kdev (we agreed to argue about spelling alphabets via PN and dont bother the development with stuff like this). What is the current status of this PR? Looking forward to see it ingame 😊

@buffcode
Copy link
Contributor Author

Despite my argument with kdev (we agreed to argue about spelling alphabets via PN and dont bother the development with stuff like this). What is the current status of this PR? Looking forward to see it ingame 😊

I don't have so much time currently. I tried to implement the template preview, but the Settings Vue Component doesn't seem to be compatible with the composition API currently, at least I was unable to satisfy Typescript, though the project could be build.

I have a v3 import feature locally, that enables a button to import existing settings from renameFz when the respective localstorage item is detected. If wanted I can add it to this PR as well.

@jxn-30
Copy link
Member

jxn-30 commented Feb 29, 2024

Typescript doesn't check within vue files while compiling and building LSSM currently.
As long as it works, it's currently okay if Typescript would fail. We will have to have a look at all the vue files after migrating to vue3, so till than a more or less hacky variant is okay in my eyes.

TypeScript Support will also get better when all stores are migrated to setup stores (and settings store is one of them that needs full rewrite), so I hope that maybe we will even reach a point where LSSM-Settings are fully typed, which allows perfect type checking within setting components. But that is just future thinking ^^

The import button would be very nice, push is welcome :)

@jxn-30
Copy link
Member

jxn-30 commented Feb 29, 2024

Just to be clear: I am not happy with hacky solutions being necessary but I expect them to be much less necessary in the future (hopefully not too far away) ^^

Try are still allowed as long as not-hacky ones are frustratig

@buffcode buffcode marked this pull request as ready for review February 29, 2024 22:39
@buffcode
Copy link
Contributor Author

Typescript will fail with:

### [🚨] check TypeScript ###
src/modules/nameSchema/settings.ts:208:9 - error TS2741: Property 'default' is missing in type 'Omit<Custom<never, never, never, never, never, never>, "default" | "value" | "isDisabled" | "properties">' but required in type 'Omit<SettingType<unknown, Record<string, never>, DefaultData<Vue>, DefaultMethods<Vue>, DefaultComputed, DefaultProps>, "value" | "isDisabled">'.
208         importFromRenameFz: {
        ~~~~~~~~~~~~~~~~~~
typings/Setting.d.ts:172:5
172     default: AppendableListItem[];
    ~~~~~~~
    'default' is declared here.
Found 1 error.

@jxn-30
Copy link
Member

jxn-30 commented Feb 29, 2024

Hmm, I will have a look at it tomorrow. I think, that's one of the situations where it becomes very hacky 😅🙈

@Cr4zyc4k3 Cr4zyc4k3 linked an issue Mar 2, 2024 that may be closed by this pull request
35 tasks
@Cr4zyc4k3 Cr4zyc4k3 added enhancement New feature or request dependencies Pull requests that update a dependency file i18n This issue is related to i18n and locales labels Mar 2, 2024
@jxn-30
Copy link
Member

jxn-30 commented Mar 3, 2024

Found a hacky way where TypeScript doesn't complain:

importFromRenameFz: <Omit<Custom<null>, 'isDisabled' | 'value'>>({
    type: 'custom',
    component: ImportSettingComponent,
    default: null,
    properties: {},
    disabled() {
        return (
            window.localStorage.getItem('lssm_LSS_RENAMEFZ_STORAGE') ===
            null
        );
    },
} as unknown),

I know how hacky that is, but it works and improving Types of the Settings is already on my TODO :)

@Cr4zyc4k3
Copy link
Collaborator

Is there anything we can do to help you get this PR merged? 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request i18n This issue is related to i18n and locales
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Module: RenameVehicles
5 participants