sts-sponsors team mailing list archive

[Jones Ogolo <mp+442192@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

Q.A looks good, I just mentioned some few comments that need attention, otherwise we're good to go.

Diff comments:

> diff --git a/frontend/src/components/TokensList/TokensList.tsx b/frontend/src/components/TokensList/TokensList.tsx
> index 8b93285..f0e4fc6 100644
> --- a/frontend/src/components/TokensList/TokensList.tsx
> +++ b/frontend/src/components/TokensList/TokensList.tsx
> @@ -66,46 +64,22 @@ const TokensList = () => {
>        <header className="tokens-list-header" id="tokens-list-header">
>          <Row>
>            <Col size={12}>
> -            <p>Follow the enrolment steps to enrol new regions:</p>
> -            <Accordion
> -              sections={[
> -                {
> -                  content: (
> -                    <ol>
> -                      <li>
> -                        Generate single use tokens by clicking <strong>Generate tokens</strong>
> -                      </li>
> -                      <li>
> -                        Install site-manager-agent alongside a MAAS region controller
> -                        <br />
> -                        <code>snap install site-manager-agent</code>
> -                      </li>
> -                      <li>
> -                        In the site-manager-agent CLI paste the snippet below. Download the{" "}
> -                        <ExternalLink to={docsUrls.configFile}>
> -                          {/* TODO: Update link once documentation is live https://warthogs.atlassian.net/browse/MAASENG-1585 */}
> -                          optional config file
> -                        </ExternalLink>{" "}
> -                        to provide additional data for a specific MAAS region.
> -                        <br />
> -                        <code>site-manager-agent enrol location.host $ENROLMENT_TOKEN [$CONFIG_FILE_PATH]</code>
> -                        <br />
> -                        {/* TODO: Add certificate here once endpoint is ready https://warthogs.atlassian.net/browse/MAASENG-1584 */}
> -                      </li>
> -                      <li>
> -                        Accept the incoming request in the <Link to={routesConfig.requests.path}>Requests page</Link>
> -                      </li>
> -                    </ol>
> -                  ),
> -                  title: "Enrolment steps",
> -                },
> -              ]}
> -            />
> -            <p>
> -              Learn more about the enrolment process and bulk enrolment{" "}
> -              {/* TODO: Update link once documentation is live https://warthogs.atlassian.net/browse/MAASENG-1585 */}
> -              <ExternalLink to={docsUrls.baseDocsLink}>in the documentation</ExternalLink>.
> +            <p className="tokens-list-instructions">
> +              Follow the enrolment steps outlined in the{" "}
> +              <ExternalLink to={docsUrls.enrollmentRequest}>documentation</ExternalLink> to enrol new regions. Once an

The docsUrl still points to the same page, is this a placeholder?

> +              enrolment request was made use the following certificate data to compare against the certificate shown in
> +              the enrolment request:
>              </p>
> +            <code className="tokens-list-certificate">
> +              <span>CN:</span>
> +              <span>sitemanager.example.com</span>
> +              <span>Expiration date:</span>
> +              <span>Thu, 29 Jul. 2035</span>
> +              <span>Fingerprint:</span>
> +              <span>15cf96e8bad3eea3ef3c10badcd88f66fe236e0de99027451120bc7cd69c0012</span>
> +              <span>Issued by:</span>
> +              <span>Let's Encrypt</span>
> +            </code>
>            </Col>
>          </Row>
>          <Row>
> diff --git a/frontend/src/components/TokensList/_TokensList.scss b/frontend/src/components/TokensList/_TokensList.scss
> index 64694b2..ac01952 100644
> --- a/frontend/src/components/TokensList/_TokensList.scss
> +++ b/frontend/src/components/TokensList/_TokensList.scss
> @@ -1,10 +1,39 @@
>  .tokens-list-header {
> +
>    @media only screen and (min-width: $breakpoint-small) {
> -    
> +    height: 555px;
>      position: sticky;
>      top: -0.75rem;
>      background-color: white;
>      z-index: 1;
>      padding-top: 0.75rem;
>    }
> +
> +  @media only screen and (min-width: $breakpoint-large) {
> +    height: 375px;

height: 23.4375rem;

> +  }
> +
> +  .tokens-list-certificate {
> +    display: grid;
> +    width: fit-content;
> +    grid-template-columns: 1fr 1fr;
> +    grid-gap: $spv--small;
> +    overflow-x: auto;
> +    white-space: nowrap;
> +    margin-bottom: $spv--large * 2;
> +
> +    @media only screen and (max-width: $breakpoint-large) {
> +      width: 100%;
> +    }
> +  }
> +
> +  .tokens-list-instructions {

I think we're resorting to using only min-width to enable mobile-first design, this block of code could look like 
```.tokens-list-instructions {
    height: 10.9375rem;
    @media only screen and (min-width: $breakpoint-large) {
      height: auto;
    }
  }```

> +    @media only screen and (max-width: $breakpoint-large) {
> +      height: 175px;
> +    }
> +  }
> +
> +  .pagination-bar {
> +    margin-bottom: 0;
> +  }
>  }


-- 
https://code.launchpad.net/~nickdv99/maas-site-manager/+git/site-manager/+merge/442192
Your team MAAS Committers is requested to review the proposed merge of ~nickdv99/maas-site-manager:adjust-sticky-styling into maas-site-manager:main.