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

[WIP] Add example plugin hub #4634

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[WIP] Add example plugin hub #4634

wants to merge 2 commits into from

Conversation

buger
Copy link
Member

@buger buger commented May 15, 2024

User description

For internal users - Please add a Jira DX PR ticket to the subject!



Preview Link


Description


Screenshots (if appropriate)


Checklist

  • I have added a preview link to the PR description.
  • I have reviewed the guidelines for contributing to this repository.
  • I have read the technical guidelines for contributing to this repository.
  • Make sure you have started your change off our latest master.
  • I labelled the PR

PR Type

enhancement, documentation


Description

  • Introduced a new Plugin Hub page in the Tyk documentation, showcasing various plugins with descriptions and links.
  • Enhanced badge styling in SCSS for better visual presentation of plugin entries.
  • Updated HTML templates to support new styling and structure for badges, including a new bottom-title feature.
  • Configured the documentation menu to include the new Plugin Hub page for easier navigation.

Changes walkthrough 📝

Relevant files
Enhancement
_docs.scss
Enhance badge styling for plugin hub                                         

tyk-docs/assets/scss/_docs.scss

  • Added styles for centering text and setting font weight in badges.
  • Introduced new badge styling for bottom titles with specific color and
    font settings.
  • +24/-2   
    baseof.html
    Add conditional padding to page content                                   

    tyk-docs/themes/tykio/layouts/_default/baseof.html

  • Added conditional padding styles to the main content area based on
    page parameters.
  • +1/-1     
    badge.html
    Update badge shortcode for centered titles and new bottom-title

    tyk-docs/themes/tykio/layouts/shortcodes/badge.html

  • Centered title in the badge shortcode.
  • Added new bottom-title element with centered styling in the badge.
  • +8/-4     
    Documentation
    plugin-hub.md
    Create Plugin Hub page with plugin entries                             

    tyk-docs/content/plugin-hub.md

  • Created a new Plugin Hub page with badges linking to various plugins.
  • Utilized the badge shortcode for plugin entries with titles and
    descriptions.
  • +32/-0   
    Configuration changes
    menu.yaml
    Add Plugin Hub to documentation menu                                         

    tyk-docs/data/menu.yaml

    • Added a new entry for the Plugin Hub in the documentation menu.
    +48/-44 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @buger buger changed the title Add example plugin hub [WIP] Add example plugin hub May 15, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (cbc0091)

    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves multiple files and changes in both content and style, requiring a detailed review of both the functionality and the visual aspects.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    CSS Specificity: The new CSS rules added might have higher specificity which could override other styles unintentionally.

    Inline Styles: Usage of inline styles in HTML increases maintenance difficulty and reduces scalability.

    🔒 Security concerns

    No

    Code feedback:
    relevant filetyk-docs/assets/scss/_docs.scss
    suggestion      

    Consider using a class instead of inline styles for .badge .nav.bottom-title to maintain consistency and ease of maintenance. [important]

    relevant linebackground-color: $brand-light-green;

    relevant filetyk-docs/themes/tykio/layouts/_default/baseof.html
    suggestion      

    Avoid using inline styles for padding in the <article> tag to enhance CSS maintainability. Consider adding a class and defining these styles in a CSS file. [important]

    relevant line

    relevant filetyk-docs/themes/tykio/layouts/shortcodes/badge.html
    suggestion      

    Replace inline style for centering content with a CSS class in .nav.title to improve code cleanliness and separation of concerns. [important]

    relevant line
    {{.Get "title"}}

    relevant filetyk-docs/content/plugin-hub.md
    suggestion      

    Ensure that the new page content in plugin-hub.md is integrated with the site's navigation and accessible from relevant sections. [medium]

    relevant linetitle: Tyk Plugin Hub

    Copy link

    netlify bot commented May 15, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit 3063acc
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/6644f719f94ae6000877275c
    😎 Deploy Preview https://deploy-preview-4634--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Replace the non-standard CSS property with a valid one

    The CSS property text-wrap is not a standard CSS property. You might have intended to use
    white-space, word-wrap, or overflow-wrap. Please replace text-wrap with a valid CSS
    property to ensure the styles are applied correctly.

    tyk-docs/assets/scss/_docs.scss [38-41]

     .badge center p {
    -    text-wrap: balance;
    +    word-wrap: break-word;
         font-weight: 500;
     }
     
    Suggestion importance[1-10]: 10

    Why: The suggestion correctly identifies and fixes a non-standard CSS property text-wrap to word-wrap, which is crucial for the CSS to function properly in browsers.

    10
    Remove the duplicate color property to avoid styling conflicts

    The CSS rule .badge .nav.bottom-title specifies the color property twice with different
    values (white and $brand-black). This could lead to confusion and unintended styling.
    Consider removing one of the color properties to maintain consistent styling.

    tyk-docs/assets/scss/_docs.scss [100-115]

     .badge .nav.bottom-title {
         position: absolute;
         left: 50%;
         bottom: 0px;
         width: 116px;
         height: 30px;
         top: auto;
         border-radius: 18px 18px 0 0;
         text-align: center;
    -    color: white;
    +    color: $brand-black;  # Choose the appropriate color
         font-weight: bold;
         padding: 4px;
         transform: translateX(-50%);
         background-color: $brand-light-green;
    -    color: $brand-black;
     }
     
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly points out the issue of having two conflicting color properties in the same CSS rule, which can cause styling issues and confusion.

    8
    Possible bug
    Ensure HTML tags are opened and closed within the same conditional block to avoid invalid HTML

    The

    tag is opened inside a conditional block but closed outside of it. This could lead to
    invalid HTML structure if the condition is not met. Ensure that the

    tag is properly
    opened and closed within the same conditional block.

    tyk-docs/themes/tykio/layouts/shortcodes/badge.html [25-27]

     {{if .Get "title"}}
         <div class="nav title" style="justify-content: center">{{.Get "title"}}</div>
    +    <p>
     {{end}}
    -<p>
     
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential issue where HTML structure could become invalid due to conditional rendering, which is crucial for maintaining valid HTML output.

    9
    Maintainability
    Move inline styles to a CSS class to improve code maintainability

    The inline styles added to the

    tag can lead to difficulties in maintenance and override
    issues. Consider moving these styles to a CSS class in the stylesheet. This will improve
    maintainability and make the styles easier to manage.

    tyk-docs/themes/tykio/layouts/_default/baseof.html [68]

    -<article class="page-content {{if .Params.hideSidebar}}home-mobile-padding{{end}}" {{if .Params.hasPadding}}style="padding-right: 2rem; padding-top: 60px; padding-left: 30px"{{end}}>
    +<article class="page-content {{if .Params.hideSidebar}}home-mobile-padding{{end}} {{if .Params.hasPadding}}custom-padding{{end}}">
     
    Suggestion importance[1-10]: 7

    Why: Moving inline styles to a CSS class is a good practice for maintainability and easier style management, making this a valuable suggestion.

    7

    @letzya letzya marked this pull request as draft June 11, 2024 09:50
    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.

    1 participant