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

group pinned apps #996

Merged
merged 7 commits into from
Apr 7, 2021
Merged

group pinned apps #996

merged 7 commits into from
Apr 7, 2021

Conversation

johrstrom
Copy link
Contributor

Fixes #920

group pinned apps by their original category and add a heading for each section. This is essentially making the dashboard#index with pinned apps the same as apps#featured (i.e., shared apps).

It is slightly different in the way the widgets are arranged (how they interact with the MOTD and the XDMOD panels) but the partials to generate the rows in app/views/dashboard/pinned_apps are basically carbon copies of similar partials in app/views/apps/. (only these headings are h4 instead of h2)

apps#featured
image

dashboard#index
image

@johrstrom
Copy link
Contributor Author

#920 indicates configurable branding for these headers. This PR does not include that, but it can.

@ericfranz
Copy link
Contributor

So right now this groups by 100% of the time.

#920 had the idea of in the pinned_apps configuration being able to specify a single group_by: KEY where one option would be group_by: category (which would use original category) or group_by: METADATA_KEY_NAME such as group_by: field_of_science or group_by: type etc.

But if group_by: was omitted from the pinned_apps config, no grouping would occur.

@johrstrom
Copy link
Contributor Author

OK, I'll add that.

group pinned apps by a configuration allows the app admin to group
pinned apps by some known field. This also allows for safety in that
OodAppGroup#group_by now accounts for group_by methods that may exist
creating a 'not groupable' group.
@johrstrom
Copy link
Contributor Author

I've added the functionality to group by known fields (category, subcategory, owner etc). Metadata fields I can have in another PR.

Note that this adds a new config pinned_apps_group_by instead of having the key inside the pinned_apps to avoid funny marshaling or having to expand the structure of pinned_apps from an array to a map. But that can easily be changed.

</div>

<%- if Configuration.pinned_apps_group_by.present? -%>
<%= render(partial: "/dashboard/pinned_apps/group", collection: OodAppGroup.groups_for(apps: @pinned_apps, group_by: Configuration.pinned_apps_group_by.to_sym)) %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note the structure of these new files in views/dashboard/pinned_apps (the pinned_apps being the new directory). I figure we're going to get a lot more views as time goes on. Just a heads up that there's a new directory.

@johrstrom johrstrom added the ready label Apr 6, 2021
@ericfranz ericfranz merged commit f27530a into master Apr 7, 2021
@ericfranz ericfranz deleted the group_pinned_apps branch April 7, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a group_by option to pinned apps
3 participants