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

Create details component #24

Closed
18 tasks done
Tracked by #19
jholt1 opened this issue Oct 8, 2021 · 6 comments · Fixed by #90
Closed
18 tasks done
Tracked by #19

Create details component #24

jholt1 opened this issue Oct 8, 2021 · 6 comments · Fixed by #90
Labels
Area: Components Work related to creating or updating components Status: Completed Nothing further to be done with this issue. Awaiting to be closed
Milestone

Comments

@jholt1
Copy link
Contributor

jholt1 commented Oct 8, 2021

Outcome

To create a component that shows and hides content related to a heading

Scope

Properties

  • type
  • open

Tokens

  • icon-open
  • icon-close
  • icon-position
  • type

Slots

  • heading
  • anonymous

States

  • hover
  • focused
  • active
  • default open
  • default closed

Events

  • toggle

Mixins

  • Detail Mixin

HTML Templates

  • heading
  • content / panel
  • icon
@jholt1 jholt1 added the Area: Components Work related to creating or updating components label Oct 8, 2021
@jholt1 jholt1 changed the title Create dialog component Create details component Nov 1, 2021
@PtitBen PtitBen added the Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. label Dec 2, 2021
@jholt1 jholt1 linked a pull request Dec 6, 2021 that will close this issue
2 tasks
@RobTobias123
Copy link
Contributor

As discussed with DRew, here's the cues I've created for the different states and scenarios for an expander component, that could make up an accordion from multiples of the. There are default, hover and focus states, text, icon start, icon end, and RTL variants.

Screenshot 2021-12-07 at 13 57 57

Screenshot 2021-12-07 at 13 58 19

Screenshot 2021-12-07 at 13 58 31

A couple of things to keep in mind is that some RTL languages may have issues with underlined text - such as Arabic where it might interfere with some of the lower 'dots' (I'm afraid I don't know their correct name). Also, to be aware of different line widths and line-heights due to the nature of the character set and translations. Apparantly, Arabic words cannot be shortened so FAQs would need to be written أسئلة مكررة and not FAQ.

@andij andij self-assigned this Dec 8, 2021
@andij
Copy link
Contributor

andij commented Dec 8, 2021

Hi @RobTobias123

Could the grey border on the top and bottom be removed? I ask due to the extra CSS complexity these borders introduce.

@RobTobias123
Copy link
Contributor

Hi @andij, I've modified the Skecth file to be without the top and bottom separators as you mentioned to see how it might work on context – and I think it works OK as we have retained the spacing. I also took a look at the Gov design system to see how they do things (as it's B&W and simple) and they put the heading in focus state once clicked. I've taken the same idea and done it Muon style here. What do you think? Do we need to add the background colour back in or is this simpler method better?

Screenshot 2021-12-08 at 11 36 00

@RobTobias123
Copy link
Contributor

RobTobias123 commented Dec 10, 2021

@andij here's an example of one with the heading wrapping. The open paragraph of text below maintains its distance from the bottom of the last line of the heading whilst 1st line of heading remains centrally aligned with icons (ie. in the same space that a single line heading would be). It wraps at 10px before the chevron icon's bounding box (36px sq).

Screenshot 2021-12-10 at 16 33 28

heading is 21px on 28 line-height in this example.

@andij
Copy link
Contributor

andij commented Dec 10, 2021

Hi @RobTobias123, with regards to the note you added above about the focus state being present when the heading is clicked. (and citing: https://design-system.service.gov.uk/components/accordion/)

I have implemented a technique to avoid the focus state being displayed when a user interacts using a mouse pointer or touch. I am leveraging a recent pseudo class :focus-visible.

The only browser that does not support this feature (at the time of writing) is Safari. But it's available behind a feature flag. (Which indicates that it will be released in the future.) Let's check the state of the browsers at the time Muon is about to be used in production to see if this has been implemented in Safari. https://caniuse.com/?search=focus-visible

This allows us to avoid the traditional :focus state and only display a focus state when the interactive element is triggered with a device other than a mouse pointer or a touch.

References:
https://matthiasott.com/notes/focus-visible-is-here
https://www.tpgi.com/focus-visible-and-backwards-compatibility/
https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible

@MekalaNagarajan-Centrica MekalaNagarajan-Centrica added the Status: Review Needed The issue has a PR attached to it which needs to be reviewed label Jan 5, 2022
@RobTobias123
Copy link
Contributor

FYI - I have updated the design in the Muon Sketch Library to use the sans-serif fonts and the functional icon has the same colour behaviour as links and buttons now too. The 'question' part has been H3 from the start, but I've not underlined it in this version on hover etc as it's not a 'link' style. – This appears to be consistent with other design systems out there (eg. Carbon) although we have removed the background colour change on hover - so will this still be enough indication with just the icon at Muon level? Personally, I think so, as it has a focus state and a functional icon - these should be enough visual cues for affordance and it will most likely be modified when branding is applied by the consumer.

@50% scale to fit screenshot...

Screenshot 2022-01-10 at 13 53 43

Screenshot 2022-01-10 at 13 53 51

@andij andij added Status: Completed Nothing further to be done with this issue. Awaiting to be closed and removed Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels Jan 11, 2022
@PtitBen PtitBen added this to the Beta release milestone Jan 13, 2022
@PtitBen PtitBen added Status: Review Needed The issue has a PR attached to it which needs to be reviewed and removed Status: Completed Nothing further to be done with this issue. Awaiting to be closed labels Jan 13, 2022
@andij andij added Status: Completed Nothing further to be done with this issue. Awaiting to be closed and removed Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Components Work related to creating or updating components Status: Completed Nothing further to be done with this issue. Awaiting to be closed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants