Skip to content

GH-75: Data List - Plugin#308

Merged
wesleyboar merged 19 commits intomainfrom
task/GH-75-plugin
Aug 17, 2021
Merged

GH-75: Data List - Plugin#308
wesleyboar merged 19 commits intomainfrom
task/GH-75-plugin

Conversation

@wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Aug 7, 2021 •
edited
Loading

To Do

Overview

Implement "Data List" plugin.

Issues

Changes

  • Create plugin for UI component.

Screenshots

Form
Plugin Image
Data List GH-75 Form - Data List
Data List Item GH-75 Form - Data List Itemold screenshot
Link either "Generic" > "Link" or "Bootstrap 4" > "Link / Button"
Render GH-75 Render
Structure GH-75 Structure 1 GH-75 Structure 2

Testing

Last recorded deploy to Frontera (dev) was Build #392 (Aug 9, 2021 1:02:37 PM).

Last recorded deploy to Core (dev) was Build #393 (Aug 9, 2021 1:04:25 PM).

  1. If no environment, then prepare to test CMS changes.
  2. In CMS container, run python3 manage.py migrate.
  3. On CMS website, logged in as admin, add "TACC Site" > "Data List" plugin.
  4. Within the "Data List" plugin, add "TACC Site" > "Data List Item" plugin.
  5. Ensure the following:
    1. each config value changes/adds a classname on the root element from the snippet markup (classname/config map)
    2. plugin description (next to name) in Structure view describes the required config values (short_desc/config map)
    3. labels and values added (via "Data List Item" plugins) are rendered
      • ✱ and an empty Label field results in no label text (an empty label spot)
    4. styles match those from GH-75: Data List - Styles / Component #305
      • ✱ but more space above & below between Table label/value cells
  6. Within a "Data List Item":
    1. ✱ confirm "Link" fieldset instructions render well and make sense
    2. confirm "Advanced settings" > "Support child plugin for Label" is checked
    3. † add a "Generic" > "Link" or "Bootstrap 4" > "Link / Button"‡ plugin
  7. † Confirm label becomes the link you added.
  8. Add "Generic" > "Text" plugin.
  9. In Text plugin, in "CMS Plugins" dropdown, confirm these new plugins are not available:
    1. "TACC Site" > "Data List"
    2. "TACC Site" > "Data List Item"

✱ Step added since initial review.
† Step requires testing on a branch into which task/GH-75-plugin and quick/support-plugins-unadulterated-by-bootstrap--for-main have been merged.
‡ Removed since initial review, because this plugin is not supported in this PR.

@wesleyboar
GH-75: Data List - Plugin: Container, no Items yet
03c4bb0
@wesleyboar
GH-75: Data List - Plugin: Items
7271f34 
- Add item child plugin.
- Fix classes.
- Tweak dict descriptions.
- Support tables.
- Add To Do item (add link support).
@wesleyboar
GH-75: Data List - Plugin: Support Label Link
39fc77b
@wesleyboar
GH-75: Data List - Plugin: Drop Text Support
b17c416 
Do not enable text support, because one can not nest "Data List Item".*

* Or, I found no way.
@wesleyboar wesleyboar mentioned this pull request Aug 7, 2021
Merged
@wesleyboar wesleyboar added blocked Unable to begin or continue work priority ━ Medium priority labels Aug 8, 2021
@wesleyboar wesleyboar linked an issue Aug 9, 2021 that may be closed by this pull request
"Data List" Table (for System Specs) - Styles & Plugin #75
Closed
@wesleyboar wesleyboar mentioned this pull request Aug 9, 2021
Closed
wesleyboar
wesleyboar commented Aug 9, 2021
View reviewed changes
Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Added notes.

@wesleyboar wesleyboar removed the blocked Unable to begin or continue work label Aug 9, 2021
@wesleyboar wesleyboar marked this pull request as ready for review August 9, 2021 18:47
@wesleyboar wesleyboar requested a review from iamthatian August 11, 2021 15:35
iamthatian
iamthatian reviewed Aug 12, 2021
View reviewed changes
iamthatian
iamthatian suggested changes Aug 12, 2021
View reviewed changes
Copy link
Contributor

@iamthatian iamthatian left a comment

Choose a reason for hiding this comment

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

Everything works!
I had some small python style comments.

@wesleyboar @iamthatian
Update taccsite_cms/contrib/helpers.py
041680d 
Co-authored-by: Ian Park <contact@duckonomy.com>
@wesleyboar wesleyboar added priority ▲ High priority and removed priority ━ Medium priority labels Aug 13, 2021
@wesleyboar
Copy link
Member Author

wesleyboar commented Aug 13, 2021 •
edited
Loading

@duckonomy I've added three commits (one new test step each).*

  • e224ea6 Instruct user in "Data List Item" how to add a link.
  • a7c6ae4 Allow empty Label field in "Data List Item" so user can quickly add link.
    • new test step: 5.iii.
  • 4fbf1e2 Add missing vertical padding to "Data List Item" in Table "Data List".
    • new test step: 5.iv.

* Sorry to add more after you've already reviewed. This PR is meant to support GH-88 which has a Data List component. I'd like to do it all well on the first attempt if I know how.

@wesleyboar
GH-75: Get dict value consistently
b30cbd0
@wesleyboar
GH-75: Remove AbstractMaxChildrenPlugin helper
2c174cf 
It does not seem to work, so I am removing it.

Without it, lable can be multiple links... that's okay.
@wesleyboar wesleyboar requested a review from iamthatian August 13, 2021 03:05
@wesleyboar wesleyboar mentioned this pull request Aug 13, 2021
Closed
@wesleyboar
Copy link
Member Author

wesleyboar commented Aug 16, 2021 •
edited
Loading

@duckonomy The remaining review tasks are (a) checking my resolution of your items from your previous review and (b) the three new features (sorry, these came up as I re-reviewed the UX and UI while testing integration into GH-88).

  1. Use get() not direct indexing.
  2. (Optional) Resolve list comprehension legibility.
  3. See e224ea6 and three new commits (for details).
  4. See a7c6ae4 and three new commits (for details).
  5. See 4fbf1e2 and three new commits (for details).

iamthatian
iamthatian approved these changes Aug 17, 2021
View reviewed changes
Copy link
Contributor

@iamthatian iamthatian left a comment

Choose a reason for hiding this comment

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

Functionality-wise LGTM!

I've spotted a few typos but will approve!

wesleyboar and others added 2 commits August 17, 2021 16:41
@wesleyboar @iamthatian
GH-75: Bugfix: Fix typo in user-facing text
bb5a786 
Co-authored-by: Ian Park <contact@duckonomy.com>
@wesleyboar @iamthatian
GH-75: Noop: Fix consistent typo get_short_descritpion
f2264ce 
I consistently misspelled local function name.

I did not misspell model method of the same name.

Co-authored-by: Ian Park <contact@duckonomy.com>
@wesleyboar
Copy link
Member Author

wesleyboar commented Aug 17, 2021

Thanks, @duckonomy. I've committed the changes, and tested* them.

* (via manual integration test on site/frontera/task/GH-75-plugin—cherry-picks commits from task/GH-75-plugin)

@wesleyboar wesleyboar merged commit 48e6659 into main Aug 17, 2021
@wesleyboar wesleyboar deleted the task/GH-75-plugin branch August 20, 2021 16:51
This was referenced Aug 24, 2021
Merged
Merged
@wesleyboar wesleyboar mentioned this pull request Sep 3, 2021
Closed
@wesleyboar wesleyboar mentioned this pull request Sep 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@iamthatian iamthatian iamthatian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

priority ▲ High priority

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

"Data List" Table (for System Specs) - Styles & Plugin

2 participants

@wesleyboar @iamthatian