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

/write: simplify and streamline workflow #133

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

/write: simplify and streamline workflow #133

wants to merge 19 commits into from

Conversation

aofn
Copy link
Member

@aofn aofn commented Jan 24, 2024

closes #130

@fnwbr
Copy link
Member

fnwbr commented Feb 7, 2024

🐛 Assuming that this PR is the culprit: on our current testing environment the "Create new pad" panel does not automatically close anymore, after someone clicked "Create pad". Instead it stays open.

Also, I'm missing some whitespace here:
Screenshot 2024-02-07 at 14 14 37

sort etherpad translations alphabetically and add translation
@aofn aofn marked this pull request as ready for review February 27, 2024 12:06
@aofn aofn requested a review from fnwbr February 27, 2024 12:06
@andirueckel
Copy link
Member

andirueckel commented Mar 6, 2024

@fnwbr Voilà, whitespace.

Maybe the animated service submenu approach would be even better? #96

Or would we want to open the service submenu via shadcn dialog/drawer?

Screenshot 2024-03-06 at 17 33 39

Copy link
Member

@fnwbr fnwbr left a comment

Choose a reason for hiding this comment

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

The new component feels very imbalanced in regards to whitespacing:
image

It also breaks with what the other pages do. I'm okay with changing it if the both of you prefer the more air-y, more whitespace-y version, but let's do so consistently then.


return (
<form
className="mb-8 [&>*+*]:mt-4"
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using this selector, how about adding a mt-4 to every component below? I think that would be the more correct "Tailwind"-approach.

@@ -25,6 +20,16 @@ import logger from '@/lib/Logging';
import { isMyPadsApiEnabled, path as etherpadPath } from '@/lib/Etherpad';
import { useMatrix } from '@/lib/Matrix';
import { useAuth } from '@/lib/Auth';
import CreateNewPad from './actions/CreateNewPad';

const Header = styled.header`
Copy link
Member

Choose a reason for hiding this comment

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

Can we use Tailwind instead of making these two new styled components?

@fnwbr
Copy link
Member

fnwbr commented Mar 12, 2024

Also, when the MyPads API is not enabled in the config, we still show the user the password inputs:
image

Clicking on "Create pad" errors:
image

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.

/write: simplify and streamline new item workflow
3 participants