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

[Improvement] Modal - add header actions #920

Closed
3 tasks
Tracked by #919
QuintonJason opened this issue Oct 21, 2021 · 3 comments · Fixed by #928
Closed
3 tasks
Tracked by #919

[Improvement] Modal - add header actions #920

QuintonJason opened this issue Oct 21, 2021 · 3 comments · Fixed by #928
Assignees
Labels
improvement Improve on existing work P2 Timely fix required. Fix by next release if possible.

Comments

@QuintonJason
Copy link
Contributor

QuintonJason commented Oct 21, 2021

Description

  • add create a reserved area for modal header actions
  • reposition the close button depending on the presence of header actions
  • create a rails modalHeader and modalFooter not needed

Considerations:

The header action buttons has a disabled state. This state will not exist on the button group wrapper.
For this approach, we'll add disabled to the buttons in header_actions and remove via js in the team specific domain

Design

Inactive header actions state
Screen Shot 2021-10-21 at 12 40 18 PM

Active header actions state
Screen Shot 2021-10-21 at 12 40 27 PM

Single header action
Screen Shot 2021-10-21 at 12 40 48 PM

@QuintonJason QuintonJason added the improvement Improve on existing work label Oct 21, 2021
@QuintonJason QuintonJason self-assigned this Oct 21, 2021
@QuintonJason
Copy link
Contributor Author

This seems to only be present on full screen modals

@QuintonJason
Copy link
Contributor Author

The react version is more straight forward since it already has ModalHeader, but the rails counterpart's header items exist as props on the overall modal, not a modalHeader component. Will need to update rails to align with react and most likely cause a breaking change

@QuintonJason
Copy link
Contributor Author

Regarding the above, that best case scenario. In the meantime we'll add another content_for

@philschanely philschanely added this to the Quizzing Refresh (Build) milestone Oct 23, 2021
@philschanely philschanely added the P2 Timely fix required. Fix by next release if possible. label Oct 23, 2021
@QuintonJason QuintonJason linked a pull request Oct 25, 2021 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improve on existing work P2 Timely fix required. Fix by next release if possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants