Skip to content
This repository has been archived by the owner on Aug 9, 2021. It is now read-only.

WIP Add APN policy #550

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

DIOHz0r
Copy link
Contributor

@DIOHz0r DIOHz0r commented Jun 12, 2018

Changes description

Add APN policy task to backend

Checklist

Please check if your PR fulfills the following specifications:

  • Tests for the changes have been added
  • Docs have been added/updated

References

Closes #404

imagen

@DIOHz0r DIOHz0r added this to the 2.1 milestone Jun 12, 2018
@DIOHz0r DIOHz0r requested a review from btry June 12, 2018 19:35
accesslint[bot]
accesslint bot previously approved these changes Jun 12, 2018
@btry
Copy link
Contributor

btry commented Jun 13, 2018

Hi

Looks good. Maybe we can improve the ui by showing all values in a 4 columns table (until twig becomes available in GLPI ;)).

Also we should find a better name for apn_apn.

@ajsb85 ajsb85 self-requested a review June 13, 2018 13:53
Copy link
Contributor

@ajsb85 ajsb85 left a comment

Choose a reason for hiding this comment

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

Hi, @DIOHz0r
Tests fails in PHP v7.2

@ajsb85 ajsb85 changed the title Add APN policy WIP Add APN policy Jun 13, 2018
@DIOHz0r DIOHz0r force-pushed the feature/404_apn_policy branch 2 times, most recently from d61f3a9 to 2c3eec4 Compare June 13, 2018 14:49
@DIOHz0r
Copy link
Contributor Author

DIOHz0r commented Jun 13, 2018

I've updated the code based on your comments

new UI:
imagen

@DIOHz0r DIOHz0r force-pushed the feature/404_apn_policy branch 2 times, most recently from 2b175c4 to 61d6697 Compare July 13, 2018 19:05
@DIOHz0r DIOHz0r force-pushed the feature/404_apn_policy branch 2 times, most recently from 7e620f3 to 4e1a941 Compare July 20, 2018 20:07
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

$dropdowns = ['apn_auth_type', 'apn_type'];
foreach ($dropdowns as $inputName) {
$matches = null;
preg_match('/.*<select[^>]*name=\'value\[' . $inputName . '\]\'[^>]*>.*/',
Copy link

Choose a reason for hiding this comment

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

Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.

@ajsb85 ajsb85 closed this Dec 18, 2018
@ajsb85 ajsb85 reopened this Dec 18, 2018
@ajsb85 ajsb85 changed the base branch from develop to features/for_later December 18, 2018 00:23
@btry btry closed this Dec 20, 2018
@btry btry changed the base branch from features/for_later to develop December 20, 2018 17:43
@btry btry reopened this Dec 20, 2018
@ajsb85 ajsb85 assigned DIOHz0r and unassigned DIOHz0r Dec 21, 2018
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

tpl/policy_apn_form.html.twig Show resolved Hide resolved
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

tests/suite-unit/PluginFlyvemdmPolicyApn.php Show resolved Hide resolved
@ajsb85 ajsb85 modified the milestones: 2.1, 3.0 Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure APN
3 participants