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

feat(website): add stretchable layout for playground #2067

Merged
merged 19 commits into from
Sep 21, 2024

Conversation

jerensl
Copy link
Contributor

@jerensl jerensl commented Jul 11, 2024

Overview

In the context of stretchable editor space playground, I decided to use framer-motion because they really care about accesibility and performance, why I really like working this library is how they're just giving us some utility function which can be expanded for broader use cases, I love that because it's enabled us as a developer to be more creative when dealing with animation and interaction.

For the stretchable alone I'm using useTransform, it's seems counterintuitive in the first place but after you understand it's just a pub sub-model, it's a bit clever how they were implementing it that way.

In the context of state management, I am using useReducer purely because of familiarity with the built-in state management by React, I am also using Immer for maintaining an immutability state which is an good pattern in react when the state must be immutable and not directly changed, it also enables us to work in a complex nested state, I'm also using typescript to prevent any mutation outside of useReducer

Functional Requirements

  1. User Interface Design:
  • Add a draggable handle or separator between the editor space.
  • When users hover over the handle, display a cursor indicating that the handle is draggable.
  • Implement visual feedback, such as a change in cursor appearance or highlighting, to indicate the dragging action.
  1. Resize Functionality:
  • Allow users to click and drag the handle horizontally to resize the width of the editor space.
  • Implement logic to dynamically adjust the width of the editor space and the content area as the handle is dragged.
  1. Constraints and Limits:
  • Set minimum and maximum widths for the sidebar to prevent it from becoming too narrow or too wide, ensuring a usable and aesthetically pleasing layout.
  • Consider responsive design principles to accommodate different screen sizes and resolutions.
  1. Persistence:
  • Save the user's preferred editor space width in local storage or as part of their user settings, so that it persists across sessions.
  • Ensure that the selected editor space width is maintained when navigating between pages or refreshing the browser.
  1. Accessibility Considerations:
  • Ensure that the stretchable editor space feature is accessible to users with disabilities by providing keyboard navigation options and ensuring compatibility with screen readers.
  • Implement appropriate aria roles and attributes to convey the purpose and state of the stretchable editor space to assistive technologies.

Non-Functional Requirements

  • Add framer-motion for animation and interaction
  • Add Stretchable Layout using framer-motion.
  • Integration with Playground.
  • Add bundler analyzer to detect whether the library added is implementing tree shaking or not.
  • Add measurement on the width of the container, looks like this library implemented tree shaking
  • Refactor the HTML element to use a semantic element or role-based if not possible.
  • Add state management with Immutability State using Immer for managing complex states.
  • Adding an indicator that indicates the editor codespace/options is open
  • Add a global state for the device screen type to maintain responsiveness.
  • Find the solution for storing resizable to global state with percentage not pixel units

Related Issue

Resolved #1846

Additional Notes

@jerensl jerensl requested a review from devilkiller-ag as a code owner July 11, 2024 05:57
Copy link

netlify bot commented Jul 11, 2024

Deploy Preview for modelina ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/modelina/deploys/66e9d70e4e4b6e6b5b628266
😎 Deploy Preview https://deploy-preview-2067--modelina.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@coveralls
Copy link

coveralls commented Jul 11, 2024

Pull Request Test Coverage Report for Build 10310584171

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.314%

Totals Coverage Status
Change from base Build 10077867659: 0.0%
Covered Lines: 6002
Relevant Lines: 6333

💛 - Coveralls

@jerensl
Copy link
Contributor Author

jerensl commented Jul 12, 2024

@devilkiller-ag To accommodate max/min width I need to adjust the position of the layout and put output options on the left side of the panel, also I am not sure about the number for now I put 20% minimum appearance for either input/output editor, did you have an opinion about this one?

@jerensl
Copy link
Contributor Author

jerensl commented Jul 30, 2024

@devilkiller-ag can I get a review or feedback at least? it's been a while

@devilkiller-ag
Copy link
Member

@devilkiller-ag can I get a review or feedback at least? it's been a while

Hi @jerensl Apologies for the late review. I got stuck with some work. I will do the review by tomorrow.

@jerensl
Copy link
Contributor Author

jerensl commented Jul 30, 2024

@devilkiller-ag can I get a review or feedback at least? it's been a while

Hi @jerensl Apologies for the late review. I got stuck with some work. I will do the review by tomorrow.

Take your time, I did it just for visibility because sometimes also notifications will not appear on me, and lose track of some PR. No need to hurry :)

@devilkiller-ag
Copy link
Member

@devilkiller-ag To accommodate max/min width I need to adjust the position of the layout and put output options on the left side of the panel, also I am not sure about the number for now I put 20% minimum appearance for either input/output editor, did you have an opinion about this one?

We can put a 30% minimum appearance because the input editor generally will have a deeply nested code, and its formatting keeps most code lines after 3-4 tab spaces. Can you try it if it looks good or not?

Copy link
Member

@devilkiller-ag devilkiller-ag left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. Can you change the minimum appearance for the input/output editor to 30% and check? Also, why do we need to use immer?

@@ -0,0 +1,153 @@
import type { Draft, Immutable } from 'immer';
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's one of react design's core principles which is immutability in state, sometimes when the state is not immutable it also doesn't trigger the re-rendering process, the state is updated but not changed on the UI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, object reference uses heap allocation when the object property value changes, it's still using the same memory address

@devilkiller-ag
Copy link
Member

9. Accessibility Considerations:

  • Ensure that the stretchable editor space feature is accessible to users with disabilities by providing keyboard navigation options and ensuring compatibility with screen readers.

Is it feasible to add this accessibility feature?

Again sorry for the late review, I was stuck in some work the past two weeks.

@jerensl
Copy link
Contributor Author

jerensl commented Aug 6, 2024

  1. Accessibility Considerations:
  • Ensure that the stretchable editor space feature is accessible to users with disabilities by providing keyboard navigation options and ensuring compatibility with screen readers.

Is it feasible to add this accessibility feature?

I'm not sure either, but I'm already implementing some semantic and basic aria roles as well. Just not be sure how much it helps and how to measure it precisely

Again sorry for the late review, I was stuck in some work the past two weeks.

No worries

Copy link
Member

@devilkiller-ag devilkiller-ag left a comment

Choose a reason for hiding this comment

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

image

In the mobile view, When accessing the output navigation (last icon) from editor options, it is always hidden by the opened-up editor options. So, for mobile view, I suggest either hiding its icon when the editor option is open or switching it to the output screen with the output option opened up on clicking its icon. Rest everything seems perfect to me.

@jonaslagoni what's your view on this?

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

In the mobile view, When accessing the output navigation (last icon) from editor options, it is always hidden by the opened-up editor options. So, for mobile view, I suggest either hiding its icon when the editor option is open or switching it to the output screen with the output option opened up on clicking its icon. Rest everything seems perfect to me.

@jonaslagoni what's your view on this?

I agree, never have a button unclickable ✌️

Other than that this looks good ✌️

@jonaslagoni
Copy link
Member

@jerensl seem like all this needs is removing the unclickable buttons 🤔

Signed-off-by: jerensl <54782057+jerensl@users.noreply.github.com>
@jerensl
Copy link
Contributor Author

jerensl commented Sep 14, 2024

@devilkiller-ag @devilkiller-ag, I apologize for the late response, I already made changes for mobile and hid the output options when the user clicked on the general options, what do you think?

Copy link

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@jonaslagoni
Copy link
Member

@devilkiller-ag anything from you?

@devilkiller-ag
Copy link
Member

@jonaslagoni All good from my side too!! I was just waiting for the deployment preview to work. Thanks, @jerensl for working on this issue ✨

@devilkiller-ag
Copy link
Member

/rtm

@jonaslagoni jonaslagoni merged commit 1210d14 into asyncapi:master Sep 21, 2024
16 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

[FEATURE] Add Stretchable Sidebar to Playground
5 participants