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

Transformations: Move transformation addition into drawer #78299

Merged
merged 23 commits into from
Dec 1, 2023

Conversation

codeincarnate
Copy link
Collaborator

What is this feature?

Elaboration on #77483. Was running into issues because the interleaved nature of the transformRedesign flag usage was making it difficult to clearly separate new and old interactions. This adds transformation to drawer but also moves the transformation pickers (new and old) into separate components.

Which issue(s) does this PR fix?:

Fixes #74197

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@codeincarnate codeincarnate marked this pull request as ready for review November 22, 2023 19:46
@codeincarnate codeincarnate requested a review from a team November 22, 2023 19:46
@codeincarnate codeincarnate requested a review from a team as a code owner November 22, 2023 19:46
@codeincarnate codeincarnate requested review from oscarkilhed, mdvictor, joshhunt and eledobleefe and removed request for a team November 22, 2023 19:46
Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

Took this for a spin!

I really like it, feels like a really good use of a drawer
Screenshot from 2023-11-23 15-09-04

There is a bug with the buttons, no space between them

Screenshot from 2023-11-23 15-08-50

@torkelo
Copy link
Member

torkelo commented Nov 23, 2023

I would try a 2 column layout for the transformation cards, just to try a list view that is not so dense

@codeincarnate
Copy link
Collaborator Author

Been playing with the layout a bit currently tricky though without changing up the card style. Mainly it's because the drawer width is based on the viewport so it doesn't have static sizes (with a deprecated width property as well). I'm thinking the approach for this would be expand/collapse controls mentioned here #77483 (comment). I'm not against changing up the card styles as well. What do you think @torkelo?

@codeincarnate
Copy link
Collaborator Author

Did fix the button spacing as well 😄

onSearchKeyDown: KeyboardEventHandler<HTMLInputElement>;
onTransformationAdd: Function;
suffix: ReactNode;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Copy link
Contributor

@baldm0mma baldm0mma Nov 30, 2023

Choose a reason for hiding this comment

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

Any way around these anys?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Been thinking about this one. The reason that it's any is that transformations can have, well, any structure for their options 😂 . This type had been like this before, and we can look at typing it out but it would probably require a fair amount of in-depth effort.

Copy link
Contributor

@baldm0mma baldm0mma left a comment

Choose a reason for hiding this comment

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

Great work, this looks awesome!

@codeincarnate codeincarnate merged commit b42d652 into main Dec 1, 2023
19 checks passed
@codeincarnate codeincarnate deleted the transform/drawer3.5 branch December 1, 2023 20:08
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transformations: Move transformation addition to a drawer
4 participants