GetJobber / atlantis

🔱 Atlantis
https://atlantis.getjobber.com
MIT License
27 stars 30 forks source link

fix(components): Fix SectionHeader Overlap #2090

Open ZakaryH opened 4 weeks ago

ZakaryH commented 4 weeks ago

Motivations

noticed this in another PR

image

it's from the Section List web story rendering a List with sections, inside a Card which has rounded borders

Changes

Added

Changed

Deprecated

Removed

removed a bunch of color styles that I don't believe we use anymore. seems like they existed for Icon colors, but we don't do it like that anymore

there's a single place we reference the SectionHeader.module.css and in that file we only use sectionStyles.sectionHeader nothing else.

image

Fixed

removed the background color of the section header, so that it avoids overlapping with the rounded borders of a Card

there's no need for the header to have a background color explicitly defined as far as I can tell. here I've given the background a red color and everything seems fine.

image

Security

Testing

on the Section List webstory which has a Card wrapping it, there should not be gaps in the corners of the container

the section heading background should be the same color as the rest of the List

the color of the icons in the DisplayList and SectionList should be correctly applied via the iconColor specified in the data eg.

    {
      id: 1,
      icon: "wallet",
      iconColor: "orange",
      content: "Payment for Invoice #39",
      value: "-$300.00",
      caption: "Sep 25, 2019",
      onClick: () => alert("TODO: Implement onClick"),
    },

go ahead and try a few different colors please and thank u

Changes can be tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

Random photo of Atlantis

chris-at-jobber commented 4 weeks ago

Should we strip out L41 and directly call styles.sectionHeader in L47 of List while we're here?

https://github.com/GetJobber/atlantis/blob/master/packages/components/src/List/List.tsx#L41-L47

ZakaryH commented 3 weeks ago

Should we strip out L41 and directly call styles.sectionHeader in L47 of List while we're here?

might as well. really no need for the classnames if it's always applying a single class.