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

rough out plugin-standalone logic #10672 #10678

Merged
merged 5 commits into from
Mar 15, 2024
Merged

rough out plugin-standalone logic #10672 #10678

merged 5 commits into from
Mar 15, 2024

Conversation

chrabyrd
Copy link
Contributor

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description of Change

This allows for plugins to be declared as "standalone", free from knockout and Arches chrome. Standalone plugins do not have their own js/template files and should instead be vue-only

Some things to consider:

  • currently this is reading from the config field, should is_standalone be its own column?
  • this could be DRY-ed up by, say, creating something like base_base.htm and updating base.htm to inherit from it
  • we could include support for standalone plugins to have their own .ico files
  • <title> could be updated to not have the application name, and instead just the plugin name

Issues Solved

#10672

@jacobtylerwalls jacobtylerwalls linked an issue Mar 14, 2024 that may be closed by this pull request
Knockout-less plugin template that doesn't extend base-manager.htm #10672
Closed
jacobtylerwalls
jacobtylerwalls approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Great work on this. Nothing blocking a merge from my PoV. 👍

arches/app/media/js/utils/create-vue-application.js Show resolved Hide resolved
releases/7.6.0.md Outdated Show resolved Hide resolved
arches/app/views/plugin.py Show resolved Hide resolved
arches/app/templates/views/plugin-standalone.htm Show resolved Hide resolved
arches/app/templates/views/plugin-standalone.htm Outdated Show resolved Hide resolved
arches/app/templates/views/plugin-standalone.htm
<meta name="description" content="">
<meta name="author" content="">

<link rel="shortcut icon" href="{% webpack_static 'favicons/favicon.ico' %}" type="image/x-icon" />
Copy link
Member

Choose a reason for hiding this comment

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

A nice to have would be to use the icon listed in the Plugin row as the favicon. But this can be a followup. And also depends on whether we add font awesome here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd caution against this for now. We'd need to remove support for using an .ico file and instead just drop a plain <i> tag in its place. Should be a good followup to allow users to upload a custom .ico defined in the Plugin db record.

arches/app/views/plugin.py
@@ -33,13 +33,25 @@ def get(self, request, pluginid=None, slug=None):
plugin = models.Plugin.objects.get(slug=slug)
else:
plugin = models.Plugin.objects.get(pk=pluginid)

if not request.user.has_perm("view_plugin", plugin):
Copy link
Member

Choose a reason for hiding this comment

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

In a follow-up, we might want to expose this as a config option as well. If my vue app desires to handle login itself, currently, this will redirect you to the arches auth page unnecessarily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed 👍

robgaston
robgaston approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@robgaston robgaston left a comment

Choose a reason for hiding this comment

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

@chrabyrd my quick thoughts re: DRYing up the templates: I think that it makes sense to have a separate template for this and not try to inherit what is shared. I can imagine users wanting to customize this template specifically for standalone plugins (for example, changing favicons or themes) on an individual basis, which to me implies more drift in the shared parts of this and the base-manager.htm template.

@jacobtylerwalls jacobtylerwalls mentioned this pull request Mar 14, 2024
Merged
jacobtylerwalls
jacobtylerwalls approved these changes Mar 15, 2024
View reviewed changes
Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Great!

@jacobtylerwalls jacobtylerwalls merged commit 456032e into dev/7.6.x Mar 15, 2024
4 checks passed
@jacobtylerwalls jacobtylerwalls deleted the 10672-cbyrd-vue-template-no-base-manager branch March 15, 2024 22:41
@jacobtylerwalls jacobtylerwalls mentioned this pull request Mar 15, 2024
Closed
@chrabyrd chrabyrd mentioned this pull request Mar 15, 2024
Open
jacobtylerwalls added a commit to archesproject/arches-docs that referenced this pull request Mar 18, 2024
@jacobtylerwalls
Revert "createVueApplication -> createVueApp"
87fefea 
This reverts commit 038d7eb.

Refs archesproject/arches#10678
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobtylerwalls jacobtylerwalls jacobtylerwalls approved these changes

@robgaston robgaston robgaston approved these changes

Assignees

@jacobtylerwalls jacobtylerwalls

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Knockout-less plugin template that doesn't extend base-manager.htm
3 participants
@chrabyrd @robgaston @jacobtylerwalls