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

Update menu title font size based on whether or not it's clickable #8354

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

khuddite
Copy link
Contributor

@khuddite khuddite commented Nov 5, 2024

Fixes #8350

  1. Summary
    https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=36424-125120&node-type=instance&t=psxuagFLo1Cu26Vz-0

    Based on Figma, the menu header should have base/medium instead of small/medium when it's not clickable or dropdown. However, for dropdowns, we should keep it as small/medium. Menu items should stay as base/regular.

  2. Solution
    Utilized onClick prop to decide the font size of the menu header. That way, Dropdowns stay as base/small while non-clickable headers are updated to base/medium

  3. Screenshots
    please check out font size and weight for each case

header: CleanShot 2024-11-05 at 17 45 23

menu item: CleanShot 2024-11-05 at 17 46 46

dropdown: CleanShot 2024-11-05 at 17 47 38

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 PR modifies the DropdownMenuHeader component to adjust font sizes based on clickability, implementing different sizes for clickable dropdowns versus static headers according to Figma designs.

  • Bug: StartIcon variant in packages/twenty-front/src/modules/ui/layout/dropdown/components/DropdownMenuHeader.tsx doesn't pass onClick to StyledHeader, causing incorrect font sizing
  • Font size logic in StyledHeader uses onClick prop to determine size: small (theme.font.size.sm) for clickable headers, medium (theme.font.size.md) for non-clickable ones
  • Added hover background effect only for clickable headers using onClick prop in StyledHeader component

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

github-actions bot commented Nov 5, 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 4C37:2685BE:23D826:466887:672B6A92.
    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%3Akhuddite%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'Wed, 06 Nov 2024 13:09:38 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'4C37:2685BE:23D826:466887:672B6A92'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1730898638'�[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 4C37:2685BE:23D826:466887:672B6A92.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Akhuddite%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-0ad3beb7.json

Generated by 🚫 dangerJS against c2a9d0b

@Bonapara Bonapara merged commit 5a293b8 into twentyhq:main Nov 6, 2024
17 checks passed
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.

Header should be Small/Medium
2 participants