Opened 7 years ago
Closed 7 years ago
#7612 closed enhancement (fixed)
Accessibility: Replace links with href="#" references with Semantic Elements
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Templates | Keywords: | has-patch |
Cc: |
Description
bp-nouveau: continuing work on #78 Buttons vs Links from the Github repo, I'm attaching a patch based on bp-noveau's current code in BP core.
Attachments (2)
Change History (12)
#2
@
7 years ago
- Component changed from Core to Templates
- Milestone changed from Awaiting Review to 3.0
- Type changed from defect (bug) to enhancement
#3
@
7 years ago
No, don't ask contributors to sync back to Github. If you want to do that for your own convenience, that's fine, but you'll have to do it.
#4
@
7 years ago
@mercime did you test these or was it a search/replace? Assuming the first because you've added more properties on e.g. tmpl-bp-invites-users and tmpl-bp-invites-selection. :)
Looks fine
#5
@
7 years ago
@djpaul I consider mercime a bit more than a simple contributor otherwise I wouldn't have mentioned it, but fine disregard that comment it's not important.
#6
@
7 years ago
Thank you both for your feedback :)
As long as you've checked frontend visual styles hold up with the button changes I would commit at your convenience.
@hnla changing markup from empty links to buttons might need a few CSS tweaks. I'm waiting for bp-nouveau's "gruntfile workflow and method of registering the theme package" so I can double-check before I commit anything.
where on earth does the button 'bp-remove-item' in the js-templates form.php portion of the patch actually appear on the front end site
Can't recall. Just made sure it's a button :P
did you test these or was it a search/replace? Assuming the first because you've added more properties on e.g. tmpl-bp-invites-users and tmpl-bp-invites-selection. :)
@DJPaul Yes in a way :) The patch was tested on the next template packs plugin a few months back and checked with bp-nouveau source code in core.
#7
@
7 years ago
Can't recall. Just made sure it's a button :P
Glad it's not just me then, can't seem to pin it down anywhere, wondering if it's just redundant code, but yes for moment change it up and worry about whether it's actually used some other time. :)
#8
@
7 years ago
@mercime the majority of this seems to test ok with the following comments:
- webcam capture/save & avatar crop buttons changes fine - yes styling issues but those are in BP core as styles are hardcoded to the anchor selector so hopefully simply removing
a
to leave.button
should be fine.
- Avatar/cover image delete buttons fine, some styling concern with one but not due to button element change.
- Can't comment on the post form, as those parts of the backbone template aren't making sense to me however the change to button is a valid one so...
- Invite users: Main commnet here is the anchor link will fail as not technically in a loop for bp_member_permalink to work and finding a user_id here is going to be hard, I would not bother with a link partly as moving away from this screen simply loses it and resets back to the main invites screen as there's no state preserved, you either send or have to re-select the users to invite. Maybe compliment the alt attr with our bp-tooltip and user name so that there's a visual cue to the user?
- Invite users pagination: I'm assuming here that the button change should be fine because we've screen hid the text so assume there's meant to be icon representation but not rendering any and haven't the markup to generate one on that we can hide from screen readers so this needs work.
#9
@
7 years ago
- Keywords needs-testing removed
@hnla Thank you for testing the patch. Confirmed your points above :)
- ... I would not bother with a link partly as moving away from this screen simply loses it and resets back to the main invites screen as there's no state preserved, you either send or have to re-select the users to invite. ...
I noticed that too. I found it confusing that it seems like it's now required to send a message to the members you invited to a group before those members are sent notification of the group invites because of the new "Send Invites 2" link in the Group subnav. But that's not the case, and the new subnav link disappears after you navigate to another screen other than group invites. Something to investigate in another ticket.
- ... Maybe compliment the alt attr with our bp-tooltip and user name so that there's a visual cue to the user?
Agreed. Will create another ticket for that.
7612-2.patch is specific to replacing href="#"
in bp-nouveau files. Ready to commit.
@mercime out of interest and as these JS templates drive me round the bend :) where on earth does the button 'bp-remove-item' in the js-templates form.php portion of the patch actually appear on the front end site, I'm struggling to check it.
I would also slightly like the github repo kept updated if at all possible, a few changes I've made recently I've committed on github then copied over to bp core to commit, I realise it's a hassle though.
As long as you've checked frontend visual styles hold up with the button changes I would commit at your convenience.