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

Feature/4.0 #88

Merged
merged 109 commits into from Mar 20, 2020
Merged

Feature/4.0 #88

merged 109 commits into from Mar 20, 2020

Conversation

patric-eberle
Copy link
Member

Pull request

This PR prepares the boilerplate for future projects:

Ticket

No ticket

Browser testing

Checklist

  • I merged the current development branch (before testing)
  • Added JSDoc and styleguide demo
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did run automated tests and linters
  • Did assign ticket
  • Double checked target branch

Review/Test checklist

  • Did run automated tests and linters
  • Did review code and documentation
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did check accessibility (Wave, only errors)
  • Re-assign ticket to developer

patric-eberle and others added 30 commits June 8, 2019 13:04
@patric-eberle patric-eberle added the enhancement New feature or request label Mar 18, 2020
Copy link
Collaborator

@mob8607 mob8607 left a comment

Choose a reason for hiding this comment

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

  • Very minor issue: naming of the headings is misleading because it's 700 spacing ;)

Bildschirmfoto 2020-03-19 um 11 29 19

  • There is no value handling on e-input where you can toggle the disabled state. I think this would be nice to test if it works correctly.
  • The disabled color is very light, so I think it's hard to see on many screens. Also it's different sometimes (e.g. Radio button)
  • close button in notifications seems off place:
    Bildschirmfoto 2020-03-19 um 11 13 59
  • Currently panel can't be opened or closed in styleguide. Is this on purpose?
  • No swiper gallery in styleguide
  • There are console warnings when opening the modal
  • Notifications are not displayed in modals
  • c-modal-header-01 is not displayed in styleguide
  • No margin for notifications, but I think that was already before the case

@patric-eberle
Copy link
Member Author

@mob8607

* [ ]  The disabled color is very light, so I think it's hard to see on many screens. Also it's different sometimes (e.g. Radio button)

I'll leave this for now, since it needs to be changed for each project anyway.

* [ ]  Currently panel can't be opened or closed in styleguide. Is this on purpose?

As I understand it, the state/content is handled by the parent component. It emits a click event. So I will leave this for now.

* [ ]  There are console warnings when opening the modal

Could not be reproduced. Maybe fixed with other changes.

* [ ]  Notifications are not displayed in modals

Could not be reproduced. Maybe fixed with other changes.

* [ ]  No margin for notifications, but I think that was already before the case

I'll leave this for now, since it needs to be changed for each project anyway.

@mob8607
Copy link
Collaborator

mob8607 commented Mar 20, 2020

@patric-eberle nice work!
You can merge if everything is ready from your side.

@patric-eberle patric-eberle merged commit c1663e3 into develop Mar 20, 2020
@patric-eberle patric-eberle deleted the feature/4.0 branch March 20, 2020 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants