Skip to content

Fix text sizing of feature gate table#527

Merged
ngundotra merged 1 commit intomasterfrom
fixes/feature-gate-table-bad-text
Apr 8, 2025
Merged

Fix text sizing of feature gate table#527
ngundotra merged 1 commit intomasterfrom
fixes/feature-gate-table-bad-text

Conversation

@ngundotra
Copy link
Copy Markdown
Contributor

@ngundotra ngundotra commented Apr 8, 2025

Description

Fixes text sizing

Type of change

  • Bug fix
  • New feature
  • Protocol integration
  • Documentation update
  • Other (please describe): readability

Screenshots

Testing

Related Issues

Checklist

  • My code follows the project's style guidelines
  • I have added tests that prove my fix/feature works
  • All tests pass locally and in CI
  • I have updated documentation as needed
  • CI/CD checks pass
  • I have included screenshots for protocol screens (if applicable)
  • For security-related features, I have included links to related information

Additional Notes


Important

Adjusts text sizes in feature gate table for improved readability by modifying SCSS and TSX files.

  • CSS Adjustments:
    • Added .small-headers class in _tables.scss to set font-size to 0.7125rem for table headers.
    • Applied .small-headers class to div in featureGate.tsx to adjust header text size.
  • HTML Structure:
    • Changed <h3> to <h4> in featureGate.tsx for card header title.
    • Added fs-sm class to various elements in featureGate.tsx to standardize small font size for feature titles, descriptions, and buttons.

This description was created by Ellipsis for cd93dfc. It will automatically update as commits are pushed.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer 🔄 Building (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 10:45pm

@ngundotra ngundotra merged commit 60b91b4 into master Apr 8, 2025
1 of 4 checks passed
@ngundotra ngundotra deleted the fixes/feature-gate-table-bad-text branch April 8, 2025 22:46
Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to cd93dfc in 2 minutes and 13 seconds

More details
  • Looked at 66 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. app/utils/feature-gate/featureGate.tsx:64
  • Draft comment:
    Changing the header tag from h3 to h4 may affect document outline and accessibility; ensure this change is intentional.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The rules explicitly state not to comment on pure frontend UI or styling changes. While accessibility is important, the rules assume that if the author changed the UI, they did it correctly. This appears to be purely a styling/UI hierarchy decision. The comment is also somewhat speculative with "ensure this change is intentional."
    Accessibility is a critical concern and header hierarchy does affect screen readers and document structure. Maybe this deserves special consideration beyond normal UI changes?
    While accessibility is important, the rules are clear about trusting UI changes. This is still fundamentally a UI/styling decision, and the rules state to assume the author made these changes correctly.
    The comment should be removed as it violates the rule about not commenting on UI/styling changes, and it also contains speculative language asking to "ensure" intention.
2. app/utils/feature-gate/featureGate.tsx:84
  • Draft comment:
    Ensure the class 'fs-sm' is defined in your styles so that the new text sizing is applied consistently.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure that a class is defined in the styles, which is similar to asking them to double-check something. This violates the rule against asking the author to ensure something is done.
3. app/utils/feature-gate/featureGate.tsx:98
  • Draft comment:
    Adding 'fs-sm' to the 'd-flex flex-column' container affects all its text. Verify that this is intended for a consistent font size.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to verify their intention regarding a style change. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
4. app/utils/feature-gate/featureGate.tsx:137
  • Draft comment:
    Ensure that adding 'fs-sm' to the button class for SIMD is aligned with design guidelines and maintains readability.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure alignment with design guidelines and readability, which is not specific enough. It doesn't provide a concrete suggestion or point out a specific issue with the code. It falls under the category of asking the author to double-check things, which is against the rules.
5. app/scss/dashkit/_tables.scss:31
  • Draft comment:
    Avoid using !important if possible; consider using a more specific selector or overriding variables instead to reduce potential CSS conflicts.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    While using !important is generally discouraged in CSS, there are valid use cases. This appears to be a deliberate override class specifically created to force smaller headers. The default table headers already have a font-size set (.table thead th uses $font-size-sm), so !important might be necessary here to ensure the override works. Without !important, the specificity battle might prevent the size override from working.
    The comment makes a valid general point about CSS best practices. The !important declaration could make future maintenance more difficult.
    However, this appears to be an intentional override class where !important might be necessary to achieve the desired effect, given the existing table header styles.
    The comment should be deleted as it may be suggesting a change that would break the intended functionality, and we don't have enough context to be certain that removing !important would work.
6. app/utils/feature-gate/featureGate.tsx:64
  • Draft comment:
    Changing the header element from

    to

    affects the semantic structure. Verify that this heading level is intentional for accessibility and style consistency.

  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 40% <= threshold 50%
    The comment is asking the PR author to verify their intention regarding the change in heading level, which violates the rule against asking for confirmation of intention. However, it does point out a potential issue with accessibility and style consistency, which could be useful. The comment could be rephrased to suggest checking accessibility and style consistency without asking for verification.
7. app/utils/feature-gate/featureGate.tsx:69
  • Draft comment:
    The addition of 'small-headers' on the table container and use of 'fs-sm' on text elements clearly reduces text size. Ensure these utility classes are defined globally and do not conflict with other components.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that certain utility classes are defined globally and do not conflict with other components. This falls under the category of asking the author to double-check things, which is against the rules.
8. app/utils/feature-gate/featureGate.tsx:11
  • Draft comment:
    The PR title and changes focus on text sizing in the feature gate table, but the description mentions resolving a transaction history issue. Please update the PR description to accurately match the implemented changes.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_aduJjPovbhiPrvbW


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

}

.small-headers thead th {
font-size: 0.7125rem !important;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Avoid using !important if possible; consider increasing selector specificity to override styles.

Suggested change
font-size: 0.7125rem !important;
font-size: 0.7125rem;

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.

1 participant