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

Build the enhanced plugin page #2305

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Build the enhanced plugin page #2305

merged 3 commits into from
Aug 29, 2023

Conversation

BenSurgisonGDS
Copy link
Contributor

@BenSurgisonGDS BenSurgisonGDS commented Aug 21, 2023 •
edited
Loading

See #2298

Changes included:

  • Change plugins page as per design and changes as requested
  • Support plugin config meta data
  • Add /plugins-installed route
  • Add search and filter behaviour for available plugins
  • Update Acceptance tests to handle separate Installed and Find Plugin pages
  • Added unit tests for the new post route for search
  • Cleaned up some Acceptance tests to allow them to retry correctly
  • Added meta data for the jquery proxy config
  • Added a proxy config for notify so meta data could be added

Changes not included:

  • Pagination will be done in a separate PR

@BenSurgisonGDS BenSurgisonGDS linked an issue Aug 21, 2023 that may be closed by this pull request
Build the enhanced plugin page #2298
Closed
2 tasks
@BenSurgisonGDS BenSurgisonGDS force-pushed the build-the-enhanced-plugin-page branch 11 times, most recently from 965711c to 1cb0ec0 Compare August 25, 2023 12:23
@BenSurgisonGDS BenSurgisonGDS marked this pull request as ready for review August 25, 2023 12:24
@BenSurgisonGDS BenSurgisonGDS requested a review from a team August 25, 2023 12:24
@BenSurgisonGDS BenSurgisonGDS self-assigned this Aug 25, 2023
@BenSurgisonGDS BenSurgisonGDS force-pushed the build-the-enhanced-plugin-page branch from 29f360f to 3dbdc9a Compare August 25, 2023 15:16
@nataliecarey
Copy link
Contributor

nataliecarey commented Aug 25, 2023 •
edited by BenSurgisonGDS
Loading

I'm curious about the search rules...

Would users expect step by step govuk prototype kit to come up with:

Screenshot 2023-08-25 at 16 42 53

Would users expect templates common to come up with:

Screenshot 2023-08-25 at 16 44 04

Would users expect SMS to come up with:

Screenshot 2023-08-25 at 16 45 07

These cases would be catered for if we both include the metadata in the search logic and search for each word individually rather than the complete string.

(I don't think this is necessary for a first release but I think it's worth discussing)

As this hasn't yet been exactly defined, I went with what I thought was correct for now based on the name of each plugin, not any of the meta data. What do you think @joelanman and @oli-rose28?

nataliecarey
nataliecarey reviewed Aug 25, 2023
View reviewed changes
lib/manage-prototype-handlers.test.js Outdated
@@ -33,7 +33,7 @@ const {
postPluginsStatusHandler,
postPluginsModeMiddleware,
getPluginsModeHandler,
postPluginsModeHandler
postPluginsModeHandler, postPluginsHandler
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be good to put this on a new line for consistency.

nataliecarey
nataliecarey reviewed Aug 25, 2023
View reviewed changes
lib/plugins/plugin-utils.js Outdated
assets: ['/dist']
assets: ['/dist'],
meta: {
description: 'Add the jQuery JavaScript library to your prototype'
Copy link
Contributor

Choose a reason for hiding this comment

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

This description is written as a description of what would happen if a user installs it rather than a description of what jQuery itself is. I think we should use:

jQuery is a fast, small, and feature-rich JavaScript library. It makes things like HTML document traversal and manipulation, event handling, animation, and Ajax much simpler with an easy-to-use API that works across a multitude of browsers.

(that's taken from their homepage)

@nataliecarey
Copy link
Contributor

If I try to access these pages offline it claims that everything has upgrades available, I think it should do the opposite and not claim that any have upgrades available. Screenshot attached.

Screenshot 2023-08-25 at 17 27 05

@nataliecarey
Copy link
Contributor

In the design the installed plugins also display in the "find plugins" section with a tag to say "installed". Was changing that an intentional decision?

@BenSurgisonGDS
Copy link
Contributor Author

In the design the installed plugins also display in the "find plugins" section with a tag to say "installed". Was changing that an intentional decision?

I worked with @joelanman on this. The designs were not complete and so for an MVP, this is what we have and it can be extended later.

@BenSurgisonGDS BenSurgisonGDS force-pushed the build-the-enhanced-plugin-page branch 2 times, most recently from 0707103 to 1d5968b Compare August 29, 2023 10:57
nataliecarey
nataliecarey reviewed Aug 29, 2023
View reviewed changes
lib/assets/sass/manage-prototype.scss
@@ -487,10 +488,46 @@ body .govuk-prototype-kit-manage-prototype-govuk-tag {
width: 15%
}

.govuk-prototype-kit-manage-prototype-plugin-badge{
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is unused, can you remove it please?

nataliecarey
nataliecarey reviewed Aug 29, 2023
View reviewed changes
lib/assets/sass/manage-prototype.scss
margin-left: 10px;
}

.govuk-prototype-kit-manage-prototype-plugin-updates-message {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not used (see above)

nataliecarey
nataliecarey reviewed Aug 29, 2023
View reviewed changes
lib/assets/sass/manage-prototype.scss
background-color: #fff;
}

.govuk-prototype-kit-manage-prototype-plugin-links-section{
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not used (see above)

@BenSurgisonGDS BenSurgisonGDS force-pushed the build-the-enhanced-plugin-page branch from 46af372 to f348335 Compare August 29, 2023 12:56
@BenSurgisonGDS BenSurgisonGDS merged commit b024111 into main Aug 29, 2023
27 checks passed
@BenSurgisonGDS BenSurgisonGDS deleted the build-the-enhanced-plugin-page branch August 29, 2023 14:04
@nataliecarey nataliecarey mentioned this pull request Aug 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nataliecarey nataliecarey nataliecarey approved these changes

Assignees

@BenSurgisonGDS BenSurgisonGDS

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build the enhanced plugin page
2 participants
@BenSurgisonGDS @nataliecarey