Skip to content

Feature/53585 Migrate cookie message#187

Open
jszczech wants to merge 30 commits into
developfrom
feature/53585
Open

Feature/53585 Migrate cookie message#187
jszczech wants to merge 30 commits into
developfrom
feature/53585

Conversation

@jszczech
Copy link
Copy Markdown
Contributor

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://alpaca-ui-git-feature-53585.snowdog1.now.sh

@vercel vercel Bot requested a deployment to staging May 21, 2019 09:01 Abandoned
@vercel vercel Bot requested a deployment to staging May 21, 2019 09:01 Abandoned
@vercel vercel Bot requested a deployment to staging May 21, 2019 09:02 Abandoned
Copy link
Copy Markdown
Member

@Aekal Aekal left a comment

Choose a reason for hiding this comment

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

  • In CookieMessage.js line 59 we have document, but to avoid ssr problems I think we should change it to:
const focusable = this.$root.$el.querySelectorAll('button:not([disabled]), a[href], area[href] input:not([disabled]), select:not([disabled]), textarea:not([disabled]), *[tabindex]:not([tabindex="-1"]), object, embed, *[contenteditable]')

Also this focusable list should be defined after this.isOpen = false, because now cookie itself is the first item to be focused

Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.html Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.html Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
@vercel vercel Bot temporarily deployed to staging May 21, 2019 16:10 Inactive
Copy link
Copy Markdown
Member

@Aekal Aekal left a comment

Choose a reason for hiding this comment

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

  • Please also fix hover and focus styles on button
    Zrzut ekranu 2019-05-22 o 10 17 33
  • The icon is not highlighted on button hover

Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.stories.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.scss Outdated
background: none;

.a-cookie-message__close-icon {
transition: $cookie-message__close-icon-transition;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Move this transition to &__close-icon styles, now the transition effect works only in one-way

Comment thread components/src/molecules/cookie-message/CookieMessage.scss Outdated
Joanna Szczęch added 3 commits May 28, 2019 04:42
@szafran89
Copy link
Copy Markdown
Contributor

sth wrong with line spaces, probably line-height http://prntscr.com/nvu3l7

Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.js Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.scss Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.scss Outdated
Comment thread components/src/molecules/cookie-message/CookieMessage.scss Outdated
@vercel vercel Bot temporarily deployed to staging June 4, 2019 08:30 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants