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.

Sign up for GitHub

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

Jump to bottom

fix: introduce new component Message Box #808

Merged
merged 7 commits into from Mar 31, 2020
Merged

fix: introduce new component Message Box #808

merged 7 commits into from Mar 31, 2020

Conversation

InnaAtanasova
Copy link
Contributor

@InnaAtanasova InnaAtanasova commented Mar 27, 2020 •
edited

Related Issue

Closes #807

Description

Introduce new component to the library called Message Box.
Message Box inherits the look and the behaviour of Dialog and for this reason certain blocks of code that are reused in both components are moved into _dialog-placeholders file.

@InnaAtanasova InnaAtanasova added this to the Sprint 33 - London milestone Mar 27, 2020
@InnaAtanasova InnaAtanasova self-assigned this Mar 27, 2020
@netlify
Copy link

netlify bot commented Mar 27, 2020 •
edited

Deploy preview for fundamental-styles ready!

Built with commit 550ffeb

https://deploy-preview-808--fundamental-styles.netlify.com

@Vanessa-Cusmich Vanessa-Cusmich added this to PRs Reviewer approved in Development via automation Mar 27, 2020
estelaxu3
estelaxu3 reviewed Mar 27, 2020
View reviewed changes
Copy link

@estelaxu3 estelaxu3 left a comment •
edited

Choose a reason for hiding this comment

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

I noticed couple things:

  1. The button color should highlight in blue:
    In specs:

Screen Shot 2020-03-27 at 9 19 03 AM

In Fundamental:

Screen Shot 2020-03-27 at 9 19 13 AM

  1. There are two icons in Fundamental is not documented in Fundamental:
    In specs:

Screen Shot 2020-03-27 at 9 15 26 AM

Screen Shot 2020-03-27 at 9 24 38 AM

In Fundamental:
Screen Shot 2020-03-27 at 9 15 13 AM
Screen Shot 2020-03-27 at 9 19 46 AM

Everything else looks great to me!

@estelaxu3
Copy link

estelaxu3 commented Mar 27, 2020 •
edited

For information message box, the correct icon HTML code is &#xe05c
For confirmation message box, the correct icon HTML code is &#xe090

@stefanoScalzo
Copy link
Contributor

For information message box, the correct icon HTML code is &#xe05c
For confirmation message box, the correct icon HTML code is &#xe090

The issue is that she used the icon given in the specs... So maybe we have to contact designers to update that spec?

@InnaAtanasova InnaAtanasova changed the title fix: introduce new component Message Box [DO NOT MERGE] fix: introduce new component Message Box Mar 30, 2020
@estelaxu3 estelaxu3 self-requested a review March 30, 2020 19:31
salarenko
salarenko reviewed Mar 31, 2020
View reviewed changes
Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

Nice work, I like the $message-box-states as map approach.

docs/pages/components/message-box.md Outdated
summary:
---

Message Box is used to display simple messages to the user. These messages could be Standard (Default), Confirmation, Error, Success, Warning and Information (Neutral). The Message Box component inherits the look and the behaviour of the [Dialog]({{site.baseurl}}/components/dialog.html) component.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Message Box component inherits the look and the behaviour of the Dialog

I'm not sure about that part. From my understanding Message Box is a simplified version of the Dialog + semantic states that makes a perfect fit for displaying alerts/confirmation widows etc. I don't think it will provide such features as dragging/resizing/responsiveness. Current description shows Message Box as an extension of the Dialog.

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps "...a subset of it's behavior..."

src/message-box.scss
&__content {
@extend %dialog-content;

overflow: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to prevent popovers from overflowing Message Box window. Dialog (with changes brought in this PR) doesn't prevent __content overflow.

src/message-box.scss Outdated
}

&__title {
@include fd-reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should reuse Dialogs styles for title.

@InnaAtanasova InnaAtanasova changed the title [DO NOT MERGE] fix: introduce new component Message Box fix: introduce new component Message Box Mar 31, 2020
@InnaAtanasova InnaAtanasova merged commit c8151cf into master Mar 31, 2020
Development automation moved this from PRs Reviewer approved to Done Mar 31, 2020
@InnaAtanasova InnaAtanasova deleted the fix/message-box-component branch March 31, 2020 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salarenko salarenko salarenko left review comments

@droshev droshev droshev approved these changes

@estelaxu3 estelaxu3 estelaxu3 approved these changes

@Vanessa-Cusmich Vanessa-Cusmich Vanessa-Cusmich approved these changes

@mikerodonnell89 mikerodonnell89 Awaiting requested review from mikerodonnell89

@bcullman bcullman Awaiting requested review from bcullman

@rengare rengare Awaiting requested review from rengare

@greg-a-smith greg-a-smith Awaiting requested review from greg-a-smith

@JKMarkowski JKMarkowski Awaiting requested review from JKMarkowski

@jbadan jbadan Awaiting requested review from jbadan

@stefanoScalzo stefanoScalzo Awaiting requested review from stefanoScalzo

Assignees

@InnaAtanasova InnaAtanasova

Labels
Fiori New Component
Projects
Development
  
Done
Milestone
Sprint 34 - Głogów
Development

Successfully merging this pull request may close these issues.

New component - Message Box
7 participants
@InnaAtanasova @estelaxu3 @stefanoScalzo @droshev @bcullman @salarenko @Vanessa-Cusmich