Skip to content

Comments

Fix advanced arguments help modal#728

Merged
yannickwurm merged 3 commits intowurmlab:masterfrom
tadast:tt/fixes
Feb 21, 2024
Merged

Fix advanced arguments help modal#728
yannickwurm merged 3 commits intowurmlab:masterfrom
tadast:tt/fixes

Conversation

@tadast
Copy link
Collaborator

@tadast tadast commented Feb 20, 2024

This reverts commit ecfaa3c.

The change had an unintended consequence of disabling the help
modal for advanced features.

I'd argue that #695
is a bit misleading - it is expected for HTML
labels to be clickable. Clicking on an input field label should
focus the input field, which it was doing. I don't think this should change.

It just happens that the help modal trigger is crammed into the label
too, so it can perhaps get confusing when the two elements next to each other are clickable and react to the same hover - perhaps the latter needs to change instead? Maybe the trigger icon can move to be inset on the right side of the input field?

This file should not diff after running bundle install, so perhaps
someone forgot to commit it.
This reverts commit ecfaa3c.

The change had an unintended consequence of disabling the help
modal for advanced features.

I'd argue that wurmlab#695
is a bit misleading - it is expected for HTML
labels to be clickable. Clicking on an input field label should
focus the input field, which it was doing. I don't think this should
change.

It just happens that the help modal trigger is crammed into the label
too, so it can perhaps get confusing when the two elements next to each other are clickable and react to the same hover - perhaps the latter needs
to change instead?
@ghost
Copy link

ghost commented Feb 20, 2024

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Upgrading Sinatra has broken HTTP status codes for some errors.

There was a Sinatra change sinatra/sinatra#1519
in V3 (SequenceServer has gone from v2 -> V4 recently) that made the
http status code setting from the exception class more explicit
@yannickwurm yannickwurm merged commit 89fd7a0 into wurmlab:master Feb 21, 2024
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.

2 participants