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

fix: Center Functions Empty state #7378

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

Utzig26
Copy link
Contributor

@Utzig26 Utzig26 commented Oct 2, 2024

Description

  • This PR fix Center Functions Empty state #7012
  • It changes the logic behind the rendering of SettingsPageContainer component. Now, the component is only rendered when the page content is not blank.

Changes

Before After
image image
Details

This change aligns the behavior of the settings page with the existing logic found on the `/rockets` page

Copy link

github-actions bot commented Oct 2, 2024

Fails
🚫

node failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 308F:2BA566:14EBB86:2802D7E:67007907.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3AUtzig26%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Fri, 04 Oct 2024 23:23:51 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'308F:2BA566:14EBB86:2802D7E:67007907'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1728084291'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 308F:2BA566:14EBB86:2802D7E:67007907.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3AUtzig26%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-85c02e95.json

Generated by 🚫 dangerJS against cfc5964

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request centers the empty state on the Functions page in the settings, addressing issue #7012. Key changes include:

  • Modified SettingsServerlessFunctionsTable to render SettingsPageContainer only when serverless functions exist
  • Updated SettingsServerlessFunctions to directly render SettingsServerlessFunctionsTable within a Section component
  • Aligned empty state behavior with other pages like '/rockets' for consistency
  • Improved visual layout by centering the empty state when no functions are present

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Thanks @Utzig26, LGTM!

I wonder if we should do the same on all pages, I'll confirm with @Bonapara:

  • it's a bit "weird" that the placeholder is taking full width while the page itself is not

@charlesBochet charlesBochet self-assigned this Oct 4, 2024
@charlesBochet charlesBochet merged commit c73feb5 into twentyhq:main Oct 4, 2024
11 checks passed
Copy link

oss-gg bot commented Oct 4, 2024

Awarding Utzig26: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/Utzig26

harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
## Description
- This PR fix twentyhq#7012
- It changes the logic behind the rendering of `SettingsPageContainer`
component. Now, the component is only rendered when the page content is
not blank.
## Changes
| Before | After |
|--------|--------|
|
![image](https://github.com/user-attachments/assets/98b64370-f145-41a2-a829-f86ae9687f73)
|
![image](https://github.com/user-attachments/assets/4cb0cc32-a669-4151-9444-4bc734bd2909)
|

<details><summary>Details</summary>
<p>
This change aligns the behavior of the settings page with the existing
logic found on the `/rockets` page
<img
src="https://github.com/user-attachments/assets/d2b80fbc-83e4-4823-a708-6775e19a153a"/>
</p>
</details>

Co-authored-by: Charles Bochet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Center Functions Empty state
2 participants