Giter Site home page Giter Site logo

westpacgel / gel Goto Github PK

View Code? Open in Web Editor NEW
31.0 31.0 13.0 27.1 MB

The Design System and supporting website for Westpac GEL

Home Page: https://westpacgel.netlify.com/

License: GNU General Public License v3.0

JavaScript 57.25% HTML 0.03% Shell 0.02% TypeScript 42.60% PLpgSQL 0.08% MDX 0.01%
design-system gui style-guide

gel's People

Contributors

bladey avatar borisno2 avatar chrislaneau avatar dcousens avatar dominikwilkowski avatar duwon98 avatar emmatown avatar flexdinesh avatar github-actions[bot] avatar gwyneplaine avatar jaortiz avatar jonnystening avatar jossmac avatar kenjishiromajp avatar madebymike avatar maryam-mv avatar molomby avatar noviny avatar pinktaco97 avatar samithaf avatar simonswiss avatar soobin93 avatar tomjackking avatar wojtekmaj avatar wove5 avatar wpalombini avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

gel's Issues

Component: Hint

Features

  • Tag prop
  • Spacing (via FormContext)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Fieldset

Features

  • Legend prop
  • Spacing (via FormContext)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Alert

Features

  • Responsive alert icon
  • Optional close button; leveraging Button component
  • Close functionality; fade transition
  • Inline text in XS, wrapping the icon
  • RHS padding setting optimised for close button placement
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • The text (non-box) alert type may not be required. I have a feeling alerts are only used in box style. Discuss with @justinspencer
  • Should the alert box remain in the DOM after closing? Legacy system consideration?
  • I've implemented a default Alert Icon (InfoIcon) for the Alert, since 90% of our Alerts are info alerts. The implementation allows for a different icon to be configured (icon={HouseIcon}) or removed completely (icon={null}). Is that the right approach?
  • We're going to have to update this to support Icon code-splitting, once that's made available in the icon package.

Feedback from Kate's review

  • Alert/closable - Spacing around the close 'x' on the closable alert looks a bit out (not enough padding)
    JPS: Fixed in #206
  • Alert/default - Paragraphs and different heading tag check the styling of the link in this example
    JPS: Fixed in #206

Flore review

  • Should dismiss button be moved below content in the DOM, so its tab order comes after any links?

Jonny review

  • Need to show List inside Alert, with color styling override

Component: Label

Features

  • Label as a link
  • Label as a span
  • Examples
  • Docz
  • Jest tests
  • Cypress tests

To discuss

  • Item 1

Component: Tooltip

Features

  • Text
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Should this be using abbr or span tag? Currently using span tag since abbr has a border bottom styling on it from core that has a higher specificity that I can't override.
  • Mobile approach
    • Current gel inlines the text at mobile but only for text tooltips. Have no pattern for non-text elements i.e. buttons, icons etc.
  • Should I be using react portal for this?

Feedback from Kate's review

  • tooltip/default - Question the usefulness of this component, do we need it, use it, like it???

Flore feedback

  • Tooltip is not keyboard accessible

Jonny feedback

  • If it's not being used, lets remove it
  • The positioning is a bit off
  • This is not functioning as intended ... should be able to be used as an <abbr /> element on text. Should take a Tag prop and not reset the bottom border (thats default styling of abbr)

Julie feedback

  • Show the tooltip on hover and focus
  • Programmatically set focus on the tooltip

Note: We're discussing whether this component is necessary. My guess, we'll deprecate it.

Component: Switch

Features

  • Name prop
  • Value prop
  • toggleText prop
  • Size prop: Small, Medium, Large, Xlarge
  • Block prop
  • Flipped prop
  • SrOnlyText prop
  • Checked prop
  • Disabled prop
  • onChange prop
  • Focus: isKeyboardUser mode via UserModeContext from Core
  • Accessibility consideration: aria-hidden toggle text
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • State management: Component implemented using standard CSS to style 'checked' state. Emotion is not using the checked prop to render styling. This CSS approach was taken to assist with future legacy support features.

  • Auto-generate form element IDs?

  • Should disabled state use a not-allowed cursor (as per GUI2.0)? What's best practise here?

Feedback from Kate's review

  • Switch - STG - Should be text colour label on Hero background, not white
    JPS: implemented in #206

Jonny review

  • Needs focus styling
    JPS: Fixed in #206
  • Needs explicit association (for/id value) – this came back from Intopia last time

Bolt new with multi-word component name

When bolt newing a multi-word component name the task needs to remove the dash and pascal case each of the words when it replaces the template’s _COMPONENT_NAME_ placeholder and sets a file name.

e.g. button-group ==> ButtonGroup etc

Component: Template

Features

  • Basic template
  • Template with sidebar
  • Header
    • Fixed option (responsive)
  • Footer
    • Fancy option
  • Main padded
  • Section
    • Section fill option
  • Sidebar
    • Sidebar header
    • Sidebar heading
    • Sidebar content

To discuss

  • Item 1

Component: FormRepeat

Features

  • Form utility styling
  • Help styling
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: InputCluster

Features

  • Horizontal prop: Enable horizontal mode
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • InputClusterItem

To discuss

  • Item 1

Component: FormCompacta

Features

  • Item 1
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • This component is mostly powered by the Tabcordion component. Perhaps there's a clever way we can extend that component?

Component: Dropdown

This was part of Button (along with Button-Group)

Features

  • Item 1
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

Component: ButtonWrap

Features

  • Add gap to beginning of adjacent sibling buttons
  • Examples (used in Button examples)
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Is this the best approach?

  • Is cloneElement approach, adding marginLeft via style attribute approach the best? Append to emotion css object rather than using style attribute?

  • Make generic to all inline input elements (e.g. TextInput)?

Component: Label

Features

  • Appearance prop
  • Tag prop
  • Label as a span
  • Label as a link
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • I've implemented the faint appearance variant to match the faint button styling (is color Light on default, turns to white on hover). This is different to GUI2.0's faint label styling.
  • I've followed my component based token approach as with Button. This allows for color, backgroundColor and borderColor styling for the different states of a Label (default and :hover). Is this the best approach? Do we need this level of granularity with tokens?
  • Should we combine badge and label since they are pretty much the same thing
  • Is this component actually used anywhere?
  • STG brand hero appearance uses COLORS.text for text color, other brands use white. This will be a thing throughout a number of components. The default/foreground colour mapping we used to have solved this. How do we proceed?

Feedback from Kate's review

  • No design feedback

Flore review

  • Add type="button" if Tag prop is 'button' (as per <Button />)
    JPS: implemented in #206

Component: Table

Features

  • Bordered prop
  • Responsive prop
  • Bordered
  • Bordered & striped
  • Responsive
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Table component currently only provides the parent wrapping. Row and cell (children) elements are composed with standard html elements. Are there any benefits in abstracting these standard html elements into React components?

  • Responsive table (wrapper) element enabled via prop. Maybe this should be the default? noResponsive prop to disable?

  • Table wrapper class table-responsive doesn't feel right. Used by Panel component styling; responsive table styling reset when placed inside Panel.

  • .table-highlight class doesn't feel very react-esque

  • TableWrapper approach? Should it be in the Table.js file or moved to a styled.js or TableWrapper.js (note: it's not exported)

  • Table can be composed within Panel. A .table-responsive className is rendered in case Panel composes a responsive Table (as it requires some style tweaks). Is there another way, without the className?

  • How else can we do the .table-highlight on cells/rows?

  • Do we need to check proptypes for a wrapper component that's not exported?

Feedback from Kate's review

  • Table/examples - Wrong styling for Table caption text, it's too light
    JPS: need to discuss, Dom has implemented a solution to handle system fonts not having all weights
    JPS: This restriction has now been removed

General app stuff

App features

  • Item 1

Example testing environment features

  • Rethink the sidebar. Ideally off-canvas for XS so we have an environment more aligned with the 'real world'.

  • Example environment should really utilise the Template component

To discuss

  • How is the <head> tags etc set up. Re meta tags (e.g. responsive viewport meta tag)... is this a concern of the GEL app at all, or the responsibility of the developer to implement? How do we handle this in the example environment.

  • Example environment's brand switcher uses a label wrapped radio to manage state. Labels aren't focusable in most browsers so we cant style keyboard focus for these 'buttons'. Suggest we use buttons, much like our ButtonGroup component.

  • Should we auto-generate form element IDs?

  • Should we always pass our Emotion CSS objects through facepaint mq() even if the passed values don't include array values? This would ensure if a token is updated to use an array value it would style responsively as expected. Otherwise our component(s) would have to be updated, bumped and re-published to support the token update 🤕

  • Packages added via bolt/yarn add ... are making package.json fail CI (due to formatting)

Assumptions

  • We should typecheck every component's props, whether its a simple style component (in src/styled.js) or a child component that receives props from a parent. Check all.

  • We should use HTML elements (e.g. checkbox/radio) and CSS selectors (e.g. :checked) to handle the styling of component state. e.g. Checkboxes, radios, switches etc. This will help us power our legacy (non-react) version.

Component: ProgressBar

Features

  • Value prop
  • Skinny prop: Enables skinny mode
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Currently, the progress bar only caters for percentage unit, will there be cases of other unit types?

Feedback from Kate's review

  • Progress bar/default - STG - Should be text colour label on Hero background, not white (see large progress bar % complete label)
    JPS: implemented in #206
  • General comment for the back log to review the usefulness of this component.

Jonny review

  • Missing visually hidden text in skinny
    JPS: fixed in #206

Component: Breadcrumb

Features

  • Default
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Should we render the separator icons as background images?
  • Will require code-splitting of ArrowRightIcon once Icon can support that

Feedback from Kate's review

  • Breadcrumb/default - Only a note to say that the styling of this element may change, there has been a discussion around the chevrons changing to text coloured forward slashes - this has not been confirmed yet.

Component: TextInput

Features

  • Size prop: Small, medium, large, xlarge (or via FormContext)
  • Width prop: 2, 3, 4, 5, 10, 20, 30
  • Invalid prop
  • Tag prop: Input, select, textarea
  • Disabled
  • Focus styling
  • Inline (via FormContext)
  • Fieldset wrapping
  • :focus outline styling (for all, not just keyboard users)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Select element: Custom styling of the dropdown icon... can render as data-uri SVG background, or pseudo-element if we use a wrapping div (which will be more verbose to implement for legacy version). Note: this icon is color ’Muted‘ (a colour that changes per brand). Will need to recolour this background SVG to suit each brand. Emotion makes this easy, but will need a solution for legacy version. Store SVG in the component and convert to data-url before rendering as background in emotion? Note: This has been implemented via background data-uri method using a simple dependency. Approach also used in List component.

Feedback from Kate's review

  • textInput - Need to remove all placeholder text, we do not recommend using as it is not accessible.
    JPS: Placeholder text only there as a label to indicate the size of the input for testing purposes. We need to allow placeholder since it's a valid html attribute. We will educate re its use via our documentation

Component: ButtonGroup

Features

  • Appearance prop
  • Size prop
  • Block prop
  • Name prop
  • Icons
  • Button group text (similar to switch label) – update ListGroup example once implemented
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • ButtonGroupButton
    • Value prop
    • iconAfter prop
    • iconBefore prop
    • SrOnlyText prop
    • Checked prop
    • onChange prop

To discuss

  • Currently setting button's borderRadius and border styling with CSS. Consider doing this programmatically, iterating children and applying borders as needed? Similar to InputGroup question.

  • We should probably limit the button appearance options. Do we even allow other appearances or is hero appearance it?

  • Had a thought. This could really easily be merged into the FormCheck component; provided via prop type="button" (adding to radio and checkbox types). Thoughts? @simonswiss @dominikwilkowski

Feedback from Kate's review

  • Button group - See Buttons #40

Jonny review

  • Missing hidden radio inputs

Component: ProgressRope

Features

  • Item 1
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Feedback from Kate's review

  • Progress rope/item - No design feedback

Jonny feedback

  • Missing high-contrast mode styling
  • Focus styling doens't match GUI2
  • Nav items should be buttons (currently 's without hrefs — not valid html)

Component: FormSection

Features

  • noPadding prop
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • FormSectionImg

To discuss

  • Item 1

Component: List

Features

  • Appearance
  • Type: Bullet, link, tick, unstyled, icon, ordered
  • Spacing: Medium, large
  • Icon
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Can users pass in colors to icons in lists?

Bugs

  • Have issue in docs with children prop mapping not working correctly so nested colors are incorrect
    Screen Shot 2019-07-23 at 9 54 53 am
  • How to handle children proptype checking. Having issues checking via
children: PropTypes.arrayOf(
        PropTypes.shape({
            type: PropTypes.oneOf([ListItem]),
        })
    )

Feedback from Kate's review

  • No design feedback

Jonny review

  • Ordered list indent has been reduced (compared to GUI2). Intentional?

Component: FormPod

Features

  • Header: center (responsive)
    • Pre-heading
    • Heading
    • Icon
  • Panel
    • Body
    • Footer
      • ContactList
      • Indicator
  • Actions
    • Primary
    • Secondary
    • Reverse option
    • ActionsText styling
  • Error summary
    • noTopBorder panel option
    • Use List component for error list
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Perhaps we could actually build the FormPodIndicator in a way that can be toggled on/off and the icon can rotate?

  • Is the slots approach the best way? (see FormPodFooter and FormPodActions)

  • Best way to provide FormPodContactList (passing data to items prop)?

Component: Form

Components

The form package provides the following components...

Related form components...

Form features

  • Size prop (passed to FormContext)
  • Spacing prop (passed to FormContext)
  • Inline prop (passed to FormContext)

Context

  • FormContext provides size, spacing and inline values consumed by the numerous form package components (and the separate TextInput component)

Components to move to own package

  • FormRepeat (+ other subcomponents): **Moved to own package. Ticket created... #90
  • FormCompacta: **Moved to own package. Ticket created... #91

To discuss

  • Should we split these form components into separate packages?
  • Not sure cross-sell is being used in production. Should we build it?

Component: FormLabel

Features

  • Sublabel prop: Enabled 'sub-label' styling
  • Tag prop
  • htmlFor prop
  • srOnly prop
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Panel

Features

  • Appearance prop: Hero, faint
  • Table children styling – TODO: discuss how we're going to style child components (see below)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • PanelHeader
  • PanelBody
  • PanelFooter

To discuss

  • Styling children: Is the cloneElement() approach the best way to style Panel's children (i.e. PanelHeader, PanelBody & PanelFooter) when access to a parent prop is required? We could add a className on the child components and select them via child selector in emotion, or use the styled component approach to target the child component (see https://emotion.sh/docs/styled#targeting-another-emotion-component)?

  • Do we need to export TableHeader and TableFooter components? Can we just take props for the header title and footer text?

  • Table wrapper class .table-responsive doesn't feel right. Panel refers to this selector.

Kate's feedback

  • Fix STG branding
    JPS: fixed in #208

Component: Symbol

Features

  • Width prop
  • Height prop
  • viewBoxWidth prop
  • viewBoxHeight prop
  • Symbols
  • Logos: Default and multi-brand
  • Accessibility features: aria-label or sr-only? 🤔
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Issues

  • Not rendering props table in docs. Not sure why 🤷‍♂️

To discuss

  • How do we feel about having GUI2.0's logos and symbols merged into this one component?

  • Is ‘Symbol’ the best name for this component?

  • As with Icon (#48)... is aria-label supported well enough by screen readers? Intopia have previously told us to use ‘sr-only’ technique for alt text

  • I've stored the multi-brand logo's SVG code as strings in a new SYMBOL token. To render this in the multi-brand components (LogoLarge & LogoSmall) I've used dangerouslySetInnerHTML on a svg group <g> tag, since that's the only way I could work out how to do it. Keen to learn the right way 🤔

  • After some thinking (I sometimes do this when running or in the shower)... perhaps the resize feature (width and height props) is something that Symbol should not be concerned with. We allow sizing of the Icon component via size prop, but that's because we mandate a set of sizes. The sizing of Symbols can be done externally I now reckon.

Feedback from Kate's review

  • symbols/all symbols - WindowsStoreSymbol missing
    JPS: Broken, but needs to be removed and replaced with the Microsoft store badge logo
    JPS: WindowsStoreSymbol logo removed

Jonny review

  • Missing multi-brand logos

Naming things is hard

The following things need a Knights Of The Round Table naming things discussion ⚔️

‘Overwrites’

Suggest changing this concept to ‘overrides’ as this is the term devs at Westpac use to refer to when they add styling/functionality on top of GEL components to make them look/behave differently than intended. We don't recommend doing this and devs know to avoid it, so providing a graceful and considered way to handle this will impress.

Calling the feature something familiar will help promote its use.

<Body />

Suggest renaming this component something more specific to its use... that being a wrapper to provide document typography styling. How about TextBlock, Text or Type?

Perhaps the @westpac/body package could be renamed @westpac/typography and along with the text wrapper component above, it also exports component versions of other type related elements such as a <Lead /> and <LinkText />?

<Text />

This component is exported by @westpac/text-input and renders a text input. It sits alongside Select and Textarea. Suggest renaming to TextInput so it's clear the component renders an input element.

Component: Grid

Features

  • Item 1
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Feedback from Kate's review

  • No feedback

Component: ChitChat

Features

  • Tag prop
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Well

Features

  • Nested styling
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Is this component actually used anywhere?

Feedback from Kate's review

  • Well/default - No design feedback

Component: ErrorMessage

Features

  • Message prop: Can take an array of messages
  • Icon prop
  • Tag prop
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Core

Features

  • Global resets
    • Box sizing reset
    • Focus reset
    • Additional resets: refer to Normalizer and Bootstrap reboot etc
  • Text styling
    • Font styling: fontSize, fontFamily, lineHeight, color etc.
    • Headings
      • Focus
    • Paragraphs
    • Definition lists
    • Abbr
    • Address
    • Blockquote
    • Mark
    • Selection styling
    • Links
      • Focus
  • Text extensions
  • Fonts
    • Font brand & body fontFamily settings
    • @font-face (webfont) declarations
  • Focus outline styling: isKeyboardUser mode using react state management, provided via Context by Form component
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Breakpoints: Do we really need to be specifying a xs: 576 breakpoint? What is that even? Isn't XS anything below the (min-width: 768px) breakpoint? Note: This has been removed

  • How to include our webfonts files in our development environment?

  • Best way to provide 'screen reader only' functionality? Does this utility functionality just become a component in its own right? 🤔_Note: This has been moved to the accessibility-helpers package as the SrOnly component_

  • Do we still need to differentiate between keyboard user navigation and mouse navigation. We are customising the focus styling, so will see keyboard outline styling appear on all UI controls (inputs, buttons, links etc). GUI2.0 uses .is-keyboarduser on the body to handle this. Note: isKeyboardUser mode has been implemented via Context provided by Form component

  • isKeyboardUser mode has been implemented using React state management. Should isKeyboardUser mode be managed in code via state or via body class (as per GUI2.0)? Both? How will this map across into the legacy version?

Component: FormCheck

Features

  • Type prop: Checkbox or radio
  • Name prop
  • Size prop: Medium or large
  • Inline prop
  • Flipped prop
  • Inline
  • Disabled (via attribute and via fieldset)
  • Focus
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • FormCheckOption
    • Disabled
    • Checked
    • Value

To fix

  • State management (as with ButtonGroup and Switch). Checked state example is broken. Need direction from TM re best way to do this. @Joss has some neato tricks up his sleeve.
  • Radios checking seems to be broken, likely related to the above. First click doesn't check, second click does

To discuss

  • Should disabled state use a not-allowed cursor as per GUI2.0? What's best practise here?
  • Component should ideally be implemented via composition or data structure
  • Currently doesn't consume inline via FormContext (need to decide if this is necessary)

Jonny review

  • Ticks aren't sized/positioned correctly

Component: FormGroup

Features

  • Primary prop: Enabling 'fork' pattern
  • Spacing (via FormContext)
  • Inline (via FormContext)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Item 1

Component: Popover

Features

  • Item 1
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Have issue with positioning the popover using portal. Currently acts fixed on scroll, not sure how to fix.

Feedback from Kate's review

  • Popover/default - Also need to have the dismissible version
  • Popover/default - Dismissible popover needs to have a close 'x' added in the top right

Jonny review

  • Should not lock scroll when open. Must be able to document scroll with Popovers open
  • Popover must receive focus programmatically when opened
  • Needs a close button and dismissible as a feature (currently the default)
  • Currently not handling tabbing between open popovers, page scrolls but popover remains open (fixed in the place it was last open)

Julie feedback

  • Close button at the bottom of the popover bubble in the DOM (after the content). Visually position top right.
  • Use role="tooltip" on the popover bubble
  • Association via aria-describedby="{ID}" on button and id="{ID}" on popover bubble
  • Set focus on bubble when it shows, subsequent tab goes to any tabbable elements within content, then to close button, then off the bubble and back to the toggle button (not to the end of the page, as will happen if we don't trap focus somehow). This is the ideal. If we can do this we will be rockstars 🤘

Desired behaviour

Default popover
☑️ Click button toggle to open/close
☑️ Click X close button to close
☑️ Press Esc to close
⛔️ Click page to close

Dismissible popover
☑️ Click button toggle to open/close
☑️ Click X close button to close
☑️ Press Esc to close
☑️ Click page to close (edited)

Brand tokens

Features

  • Item 1

To discuss

  • What is the expected usage of tokens?

  • Color: Maybe this dependency should somehow live in Core?

  • Color: To save lines... we could create a tint(color, amount) function?

Component: Button

Features

  • Appearance prop
  • Size prop: Small, medium, large, xlarge
  • Soft prop
  • Block prop
  • Trim prop
  • iconAfter prop
  • iconBefore prop
  • Justify prop
  • SrOnlyText prop
  • Tag prop
  • onClick prop
  • Disabled
  • Active
  • Responsive block
  • Responsive size
  • Button dropdowns – this may not be required??
  • ButtonWrap? Best approach?
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Inline button ButtonWrap component? Make generic to wrap all inline inputs (incl. TextInput)? Append marginLeft to child elements individually?

  • Are button dropdowns required?

  • Our approach with this has been to maximise the use of tokens; but since Button is quite complex, it can take numerous prop options (incl. appearance and soft) plus requiring :hover and :active state styling, it has made for a rather verbose token structure. Verbose tokens yes, but much simpler component code (and the power of publicly exposed button styling). Thoughts?

  • Icons: Buttons take Icon children. How are we best to style these? I've taken a leaf from Atlaskit's book, using iconAfter/iconBefore props. Simple enough.

  • Do we actually need to be able to take a tag="input" prop? We could simplify the codebase a little if this requirement was dropped and Buttons are either <button>, <a> or <span> (but not <input>). Nb. <input> elements add code complexity because they can't take children elements (they use a value attribute). Worth mentioning... they can't take Icons.

  • Active styling is now leaning on pointer-events: none to handle styling for active plus hover (ie. we don't want hover styling applied on an active button). Are there any issues with this? Or should our solution handle this without use of this property (ie. using a :hover:not(:active) styling).

  • Removed .disabled styling (via class name). Disabled styling only provided via disabled attribute. You can no longer disable a <a> button (<a> can't take a disabled attribute); but really probably shouldn't be doing this anyway as it's bad practice to disable links.

Feedback from Kate's review

  • Button general comment - Need to update the name of the 'Faint' button to 'Light' to align the naming convention.
    JPS: probs best to wait until we decide on primary/accent naming
  • Button/look - Text button within text not needed
    JPS: removed
  • Button/group - BACKLOG - Is it possible to add a button group type that makes the buttons of equal size regardless on the button label length? ie all match the width of the largest?
    JPS: not really, large-ish effort and code bloat for something that doesn't really work well for long button names anyway because they don't wrap. We should be using radios for nearly all instances really

Component: InputGroup

Features

  • Implementation via data
  • Size
  • Types
  • Look
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

Sub-components

  • Left (props: type, data)
  • Right (props: type, data)

To discuss

Feedback from Kate's review

  • No design feedback

Flore feedback

  • Addon on right missing border radius

Component: Tabcordion

Features

  • Default
  • Always tabs
  • Always accordion
  • Responsive ('tabcordion' mode)
  • Borderless
  • Appearance: Soft & Lego
  • Justify mode
  • Tab and accordion transition animation
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Need to work out if we like the lego accordion styling (active state seems the inverse of what it should be)
  • Accordion panels to not auto-close (multiple may be open at once) — new behaviour

Jonny review

  • Open/close needs to animate
    JPS: Will come back to this (animation library has SSR concern)
  • Keyboard tabbing functionality isn't as per GUI2
    JPS: Now fixed
  • Borderless option is missing

Component: Icon

Features

  • Size: Xsmall, small, medium, large, xlarge
  • Responsive sizing (via array value supplied to size prop)
  • Accessibility features: aria-label/sr-only?
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Would it be more conventional to name these components 'Icon' followed by the name ie. IconHouse, IconInfo, IconAlert (rather than HouseIcon, InfoIcon, AlertIcon)

  • As with Symbol (#64)... is aria-label supported well enough by screen readers? Intopia have previously told us to use sr-only for alt text

Feedback from Kate's review

  • Icons - No feedback

Component: Modal

Features

  • Sizing
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Whether separate modal header and body components are necessary
  • Use of GEL Button for the close icon

Feedback from Kate's review

  • modal/default - Doesn't seem to have all styling applied, ie Hero coloured header separator line. _JPS: This has now been fixed... In the Modal PR (#199), awaiting merge)

Jonny review

  • Currently in @jaortiz ’s PR #199. Will wait before giving to Intopia.
    JPS: now merged

Component: Badge

Features

  • Appearance prop
  • Badge inside Button: Badge styling is adjusted when the Badge is the child of a Button
  • Badge inside List: Required??
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Are badges inside lists a thing?
  • Should Badge and Label be combined into one component?
  • STG brand hero appearance uses COLORS.text for text color, other brands use white. This will be a thing throughout a number of components. The default/foreground colour mapping we used to have solved this. How do we proceed?
  • Badge styling is not yet adjusted when the Badge is the child of a Button, this feature was in GUI2.0

Feedback from Kate's review

  • Badge/links - Review styling of link text, wrong colour
    JPS: Fixed
  • Badge/buttons - Padding missing between Button label and Badge (we will need to curate this dramatically for our examples in GEL3.0)
    JPS: yeah we left this for now, not quite sure of the best way to implement this

Component: Pagination

Features

  • Disabled prev/next buttons
  • Active button styling
  • Alternative text (SrOnly)
  • Examples
  • Jest tests
  • Cypress tests
  • Docs

To discuss

  • Flore: I am not sure how the pagination items should be presented to the developer... Currently, it is similar to Breadcrumbs: each item is placed in reading order...
  • SrOnly used to provide alternative text, likely use case to use aria-label
  • Should we build in more control as to the item and link components? The current approach leaves Item to receive any children, but when we look at SrOnly features it puts more onus on the dev to implement screen reader support. Perhaps a separate First/Last component, or we provide SrOnly via props? Will there always be a 'Back' and 'Next' button? Will they always be called that? 🤔
  • We should probably be passing in a current prop to and determining the active item from that, rather than passing an active prop to . Would make it easier to update the current page at the application level this way.

Feedback from Kate's review

  • Pagination/default - No design feedback

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.