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

Fix #4030: add copy module option for add existing module function. #4198

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

zyhfish
Copy link
Contributor

@zyhfish zyhfish commented Apr 27, 2024

Fix #4030: add copy module option for add existing module function.

@sbwalker
Copy link
Member

sbwalker commented Apr 29, 2024

@zyhfish my preference for implementing this feature is that we keep the UI very simply and consistent with the approach used currently. I would simply add a "Copy Existing Module" option to the first option in Module Management:

image

This new option will behave similar to "Add Existing Module" ie. it will allow you to select a Page/Module:

image

However, copy is only possible if a module implements IPortable - so the list of modules should either be filtered so that modules which do not support IPortable are not displayed (ie. using module.ModuleDefinition.IsPortable) or the system should allow a user to select the module and then provide feedback in the existing message area that the module does not support the ability to be copied. The first option prevents users from selecting certain modules but may result in users logging issues because they think the behavior is a bug. The second option displays all modules and provides explicit feedback to the user which offers better transparency and clarity - so is probably the better approach.

I realize that for some folks with a DNN background, the technical concepts of "sync", etc... makes sense, however Oqtane is focused on a broader audience so I would prefer to not follow the DNN patterns.

@zyhfish
Copy link
Contributor Author

zyhfish commented Apr 29, 2024

Hi @sbwalker , thanks so much for the update, the code has been updated, could you please help to check again? I also noticed that some times control panel was hidden but the mask still display, this may need open another ticket to analyze.

@sbwalker
Copy link
Member

@zyhfish I will merge this PR and make some minor adjustments

@sbwalker sbwalker merged commit a2fb728 into oqtane:dev Apr 29, 2024
1 check passed
sbwalker added a commit to sbwalker/oqtane.framework that referenced this pull request Apr 29, 2024
sbwalker added a commit that referenced this pull request Apr 29, 2024
refactor #4198 - copy existing module
@zyhfish zyhfish deleted the task/fix-issue-4030 branch April 30, 2024 02:56
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.

Enh: Add Support For Copy Existing Module to Control Panel
2 participants