Skip to content

Fix button layout of manual process form#543

Open
knottheory wants to merge 3 commits into
Project60:masterfrom
knottheory:master
Open

Fix button layout of manual process form#543
knottheory wants to merge 3 commits into
Project60:masterfrom
knottheory:master

Conversation

@knottheory

Copy link
Copy Markdown
Contributor

The new manual processing layout:

Screenshot From 2026-06-04 18-54-02

Fixes issue: #430

@mattsches
mattsches requested a review from jensschuppe June 17, 2026 13:48
@jensschuppe jensschuppe added enhancement status:needs review Code needs review and testing labels Jun 18, 2026

@jensschuppe jensschuppe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your contribution @knottheory! I could only have a quick look and wasn't able to test the PR.

I'm not sure I like the onclick attributes on the new buttons, thus not approving this PR right away. If it's just about layout, this should use a CSS-only approach.

@knottheory

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing my PR @jensschuppe. Could you elaborate on why you don't like the onclick attribute? As I see it the onlick property was not changed and only moved to a button element instead of an anchor. Let me know if I missed something there.

Usually I would agree with the css-only approach, however in this case I thought it would be easier to update the template to align with the current civi templates. This way the default civi css is applied. No new css needs to be written and the styling is more aligned with the rest of CiviCRM.

Many Thanks in advance.

@dontub
dontub requested a review from jensschuppe July 23, 2026 09:55
@jensschuppe jensschuppe linked an issue Jul 23, 2026 that may be closed by this pull request

@jensschuppe jensschuppe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you elaborate on why you don't like the onclick attribute? As I see it the onlick property was not changed and only moved to a button element instead of an anchor.

I was talking about the onclick="location.href='…'" ones instead of an <a href="…"> - but obviously, there's a lot to consider for refactoring in this template and it already has onclick attributes all over the place, so I don't want to delay the improvements this PR makes because of that.

I still didn't actually test the UI so approving the PR concept-wise solely from looking at the code - except for the leftover console.log() calls, please remove those before we merge this.

@jensschuppe jensschuppe added status:needs work There is code, but it needs additional work before it should be reviewed and removed status:needs review Code needs review and testing labels Jul 23, 2026
@knottheory

Copy link
Copy Markdown
Contributor Author

Thanks for the collaboration. I will review the code tomorrow and adjust the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement status:needs work There is code, but it needs additional work before it should be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manual processing panel layout garbled

2 participants