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

New: completion icon added to progress indicators (fixes #288) #301

Merged
merged 5 commits into from
Apr 19, 2024

Conversation

kirsty-hames
Copy link
Contributor

@kirsty-hames kirsty-hames commented Apr 16, 2024

Fixes #288

New

  • Set progress indicator variables for consistent sizing.
  • Progress indicator border and .is-selected styles added.
  • Progress indicator .is-visited icon style added. Tick icon displayed for consistency with other visited component item states.

The overall .narrative__progress size has increased from 12px to 16px. The screen shots shared in the issue have larger icons (@icon-size 1.5rem for consistency with other component item visited ticks) however this limits the amount of items we can comfortably fit visually and the preference was to keep these small.

Update

  • Display progress indicators below the image always. When _hasNavigationInTextArea is enabled, displaying the indicators within the controls limits the space available for the indicators. Instead, display these consistently, below the image, across the various layout views. See issue for discussion.
  • Progress indicator margin replaced with variable (maintains same 0.25rem value).

Dependency

Vanilla narrative.less will need updating to include color and border-color styles. I'll update the testing instructions when a Vanilla PR has been raised. Issue raised.

Testing

  1. Install issue/288 branch.
  2. Comment out .narrative__progress background-color styles in Vanilla narrative.less. For example:
  &__progress {
    //background-color: @item-color;
    border-radius: 50%;
  }

  // &__progress.is-visited {
  //   background-color: @visited;
  // }

  // &__progress.is-selected {
  //   background-color: @item-color-selected;
  // }
  1. Test the following Narrative layouts on both large and small screen widths. Screen shots added for each configuration.

In component.json, default layout:

    "_hasNavigationInTextArea": false,
    "_isMobileTextBelowImage": false,
large_default small_default

Enable _hasNavigationInTextArea:
large_nav-in-text-area

Enable _hasNavigationInTextArea and _isMobileTextBelowImage:
small_nav-in-text-area_and_text-below-image
^ mobile view (based on 375px screen width) supports 9 items before progress indicators are wrapped.

- set progress indicator variables for consistent sizing
- progress margin replaced with variable (same 0.25rem value)
- progress border and .is-selected styles added
- progress .is-visited icon style added
When _hasNavigationInTextArea and _isTextBelowImage/_isMobileTextBelowImage are enabled, the additional margin offsets the vertical display of the indicators within the controls for mobile view
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@kirsty-hames
Copy link
Contributor Author

FYI @oliverfoster and @StuartNicholls as you were involved in the issue discussion.

@oliverfoster
Copy link
Member

image

Would you be able to increase the default contrast for the completed ticks please?

@kirsty-hames
Copy link
Contributor Author

kirsty-hames commented Apr 17, 2024

image

Would you be able to increase the default contrast for the completed ticks please?

Those ticks should be white. Missing color property added in 81d7c5a.

Copy link
Member

@oliverfoster oliverfoster left a comment

Choose a reason for hiding this comment

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

image

@oliverfoster oliverfoster merged commit f41ab76 into master Apr 19, 2024
@oliverfoster oliverfoster deleted the issue/288 branch April 19, 2024 11:15
github-actions bot pushed a commit that referenced this pull request Apr 19, 2024
# [7.9.0](v7.8.1...v7.9.0) (2024-04-19)

### Chore

* e2e tests on narrative component (Issue/280) (#299) ([748f3d4](748f3d4)), closes [#299](#299)

### New

* completion icon added to progress indicators (fixes #288)  (#301) ([f41ab76](f41ab76)), closes [#288](#288) [#301](#301)

### Upgrade

* Bump ip from 1.1.8 to 1.1.9 (#300) ([b2520b0](b2520b0)), closes [#300](#300)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual item states rely on color alone
4 participants