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

[ticket/17151] Make settings forms use macros #6533

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[ticket/17151] Make settings forms use macros #6533

wants to merge 14 commits into from

Conversation

rxu
Copy link
Contributor

@rxu rxu commented Sep 18, 2023 •
edited

PHPBB3-17151

Checklist:

  • Correct branch: master for new features; 3.3.x for fixes
  • Tests pass
  • Code follows coding guidelines: master and 3.3.x
  • Commit follows commit message format

Tracker ticket:

https://tracker.phpbb.com/browse/PHPBB3-17151

@rxu rxu force-pushed the ticket/17151-2 branch 7 times, most recently from 96dfeca to 10cd704 Compare September 20, 2023 13:21
@rxu rxu marked this pull request as ready for review September 21, 2023 08:03
Derky
Derky requested changes Sep 22, 2023
View reviewed changes
phpBB/includes/functions_acp.php Show resolved Hide resolved
phpBB/styles/all/template/macros/forms/select.twig Show resolved Hide resolved
phpBB/styles/all/template/macros/forms/select.twig Show resolved Hide resolved
phpBB/styles/all/template/macros/forms/select.twig Show resolved Hide resolved
tests/functional/fixtures/ext/foo/bar/acp/main_module.php Outdated Show resolved Hide resolved
phpBB/includes/functions_acp.php Outdated Show resolved Hide resolved
@rxu rxu marked this pull request as draft September 22, 2023 17:45
@rxu rxu force-pushed the ticket/17151-2 branch 7 times, most recently from f4081f0 to 44eaab9 Compare September 23, 2023 17:59
@rxu
[ticket/17151] Make settings forms use macros
9dce82c 
PHPBB3-17151
@rxu
[ticket/17151] Fix test error
438cc94 
PHPBB3-17151
@rxu
[ticket/17151] Fix registration test error
34d0ccc 
PHPBB3-17151
@rxu
[ticket/17151] Fix tests
275a9b6 
PHPBB3-17151
@rxu
[ticket/17151] Deduplicate code for building configuration options data
20151f6 
PHPBB3-17151
@rxu
[ticket/17151] Improve radio input type handling
8ef9163 
- allow more than 2 buttons count
- allow custom buttons order
- allow custom button labels

Implemented by allowing JSON data format, backward compatibility preserved.

PHPBB3-17151
@rxu
[ticket/17151] Fix PHP warning on board settings page, add ACP pages …
1c3faaf 
…test

PHPBB3-17151
@rxu
[ticket/17151] Adjust template output formatting
ddfd6e7 
PHPBB3-17151
@rxu rxu force-pushed the ticket/17151-2 branch 2 times, most recently from 3565d80 to c3bc6a3 Compare September 23, 2023 18:37
@rxu rxu requested a review from Derky September 24, 2023 08:59
@rxu rxu marked this pull request as ready for review September 25, 2023 00:45
@Derky
Copy link
Member

Derky commented Oct 1, 2023

Thanks for updating the PR! I've also asked @marc1706 to take a look at the radio options to see if it's aligned with his recent changes to form elements.

I've been testing this PR around a bit and the board settings page is working again now, but I noticed there are no selected="selected" attributes on both the "Default style" as "Guest style". So this causes the first option in the selectbox to show as selected. (So if you select a different style, it doesn't show that as selected next time you return to the page)

@rxu
Copy link
Contributor Author

rxu commented Oct 2, 2023 •
edited

there are no selected="selected" attributes on both the "Default style" as "Guest style".

At what page?

EDIT: okay, I see the cause. Will be fixed in next commits.

@rxu
Copy link
Contributor Author

rxu commented Oct 2, 2023

@Derky Probably fixed "selected" issue, could you please retest it again.

marc1706
marc1706 requested changes Oct 2, 2023
View reviewed changes
phpBB/includes/functions_acp.php Outdated Show resolved Hide resolved
phpBB/includes/ucp/ucp_prefs.php Outdated Show resolved Hide resolved
phpBB/includes/ucp/ucp_prefs.php Outdated Show resolved Hide resolved
phpBB/includes/acp/acp_board.php Outdated Show resolved Hide resolved
phpBB/includes/functions_acp.php Outdated Show resolved Hide resolved
@Derky
Copy link
Member

Derky commented Oct 2, 2023

@Derky Probably fixed "selected" issue, could you please retest it again.

Yes this was fixed, thanks!

@rxu rxu force-pushed the ticket/17151-2 branch 4 times, most recently from dd23e05 to 372b5f7 Compare October 3, 2023 05:28
@rxu
Copy link
Contributor Author

rxu commented Oct 3, 2023

@marc1706 Done.

@marc1706 marc1706 added this to the 4.0.0-a1 milestone Oct 5, 2023
@rubencm rubencm mentioned this pull request Feb 19, 2024
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Derky Derky Derky requested changes

@marc1706 marc1706 marc1706 requested changes

Assignees
No one assigned
Labels
4.0 (Triton) 🔮
Projects
None yet
Milestone
4.0.0-a1
3 participants
@rxu @Derky @marc1706