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.

Sign up for GitHub

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

Jump to bottom

Make the primary, flow-advancing action the right-most #7594

Merged
merged 3 commits into from Jul 31, 2018
Merged

Make the primary, flow-advancing action the right-most #7594

merged 3 commits into from Jul 31, 2018

Conversation

tacotexmex
Copy link
Contributor

This should be a no-brainer. UX standards dictates that the primary action should be put to the right.

@paramat
Copy link
Contributor

paramat commented Jul 28, 2018

I support this, but the button co-ordinates have not been changed?

@paramat paramat added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements @ Mainmenu labels Jul 28, 2018
ClobberXD
ClobberXD reviewed Jul 29, 2018
View reviewed changes
builtin/mainmenu/tab_local.lua Outdated
"button[9.2,3.95;2.5,1;world_configure;".. fgettext("Configure") .. "]" ..
"button[6.5,3.95.15;2.8,1;world_create;".. fgettext("New") .. "]" ..
Copy link
Contributor

@ClobberXD ClobberXD Jul 29, 2018 •
edited

Choose a reason for hiding this comment

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

Merely re-ordering the lines has no effect whatsoever. :)

I think it's just a matter of swapping the button names (world_create and world_configure) and the display names, while keeping the x, y, w, h values intact.

Copy link
Contributor

Choose a reason for hiding this comment

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

^ That.

@paramat paramat added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jul 29, 2018
@tacotexmex
Copy link
Contributor Author

Holy mese I’m stupid. xD Let me correct that later.

@SmallJoker
Copy link
Member

Indeed, that's a little too much of a "no-brainer". Although, the concept is fine.

@paramat paramat removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jul 31, 2018
@paramat
Copy link
Contributor

paramat commented Jul 31, 2018 •
edited

Did you test it? Will +1 if so.

@ClobberXD
Copy link
Contributor

ClobberXD commented Jul 31, 2018 •
edited

Haven't tested, but the code seems alright to me.

minetest/builtin/mainmenu/tab_local.lua

Line 97 in bf45644

"button[6.5,3.95.15;2.8,1;world_create;".. fgettext("New") .. "]" ..

3.95.15? Is this intended? If not, @tacotexmex could change it to 3.95

@tacotexmex
Copy link
Contributor Author

Tested and working.

@paramat
Copy link
Contributor

paramat commented Jul 31, 2018 •
edited

Well spotted, that '.15' added to '3.95' is inconsistent with the other buttons.
Docs show these are normal co-ordinate values:

minetest/doc/lua_api.txt

Line 1962 in bf45644

### `button[<X>,<Y>;<W>,<H>;<name>;<label>]`

+1 when done.

@ClobberXD
Copy link
Contributor

Looks good to me 👍

@paramat
Copy link
Contributor

paramat commented Jul 31, 2018 •
edited

👍
@SmallJoker

SmallJoker
SmallJoker approved these changes Jul 31, 2018
View reviewed changes
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Trivial stuff, looks good.

@SmallJoker SmallJoker merged commit 6afbb06 into minetest:master Jul 31, 2018
@tacotexmex tacotexmex deleted the patch-1 branch August 1, 2018 13:22
ClobberXD pushed a commit to ClobberXD/minetest that referenced this pull request Aug 17, 2018
@ClobberXD
Make the primary, flow-advancing action the right-most (minetest#7594)
ce21f03
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 23, 2019
@osjc
Make the primary, flow-advancing action the right-most (minetest#7594)
fcfb321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paramat paramat paramat left review comments

@ClobberXD ClobberXD ClobberXD left review comments

@SmallJoker SmallJoker SmallJoker approved these changes

Assignees
No one assigned
Labels
@ Mainmenu Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements >= Two approvals ✅ ✅
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tacotexmex @paramat @SmallJoker @ClobberXD