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

Refactor DownloadModModal #881

Merged
merged 1 commit into from
Nov 29, 2022
Merged

Conversation

anttimaki
Copy link
Collaborator

  • Remove the prop that controls the modal visibility. Manage it via Vuex store instead
  • Remove the prop that defines which mod to download/update. The value is passed to Vuex store as a part of the open modal command instead
  • Remove "update all mods" prop. The value can be derived from whether there's a mod to download/update set or not
  • Since the props were removed, there's no longer a need to render duplicate modals in LocalModList and OnlineModList components

The motivation for the refactoring is to support breaking the Manager page into smaller, more manageable pieces, which in turn allows easier customization.

Refs TS-1321

@MythicManiac MythicManiac changed the base branch from standalone-category-filter-modal to standalone-category-filter-modal-rebased November 28, 2022 16:54
@MythicManiac MythicManiac changed the base branch from standalone-category-filter-modal-rebased to standalone-category-filter-modal November 28, 2022 16:54
Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

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

Clarity improvement suggestions stand, looks good otherwise. Requires rebasing before approval.

src/pages/Manager.vue Outdated Show resolved Hide resolved
* Remove the prop that controls the modal visibility. Manage it via
  Vuex store instead
* Remove the prop that defines which mod to download/update. The value
  is passed to Vuex store as a part of the open modal command instead
* Remove "update all mods" prop. The value can be derived from whether
  there's a mod to download/update set or not
* Since the props were removed, there's no longer a need to render
  duplicate modals in LocalModList and OnlineModList components

The motivation for the refactoring is to support breaking the Manager
page into smaller, more manageable pieces, which in turn allows easier
customization.

Refs TS-1321
@anttimaki anttimaki changed the base branch from standalone-category-filter-modal to standalone-category-filter-modal-rebased November 29, 2022 09:22
Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Base automatically changed from standalone-category-filter-modal-rebased to develop November 29, 2022 19:26
@MythicManiac MythicManiac merged commit 15c6239 into develop Nov 29, 2022
@MythicManiac MythicManiac deleted the downloadmodmodal-refactor branch November 29, 2022 19:40
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.

3 participants