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.

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

Open link in a new window? #2204

Closed
paaljoachim opened this issue Aug 3, 2017 · 10 comments
Closed

Open link in a new window? #2204

paaljoachim opened this issue Aug 3, 2017 · 10 comments
Assignees
Labels
[Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement.

Comments

@paaljoachim
Copy link
Contributor

I am looking at creating a link.
I see no way to select having the link open in a new window.

@karmatosed
Copy link
Member

karmatosed commented Aug 4, 2017

I can replicate this, it's currently missing. I am not sure it's v1 needed but worth adding later.

@karmatosed karmatosed reopened this Aug 4, 2017
@karmatosed karmatosed added [Feature] Blocks Overall functionality of blocks Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement. labels Aug 4, 2017
@karmatosed karmatosed added this to the Beta, Nice To Have milestone Aug 4, 2017
@jwold
Copy link

jwold commented Aug 4, 2017

Took a stab at what the UI could look like. Happy for feedback!

img_930ee0334f7b-1

@notnownikki notnownikki self-assigned this Aug 31, 2017
@notnownikki
Copy link
Member

Picking this up now, based on @jwold's UI

@karmatosed karmatosed removed the Needs Design Needs design efforts. label Aug 31, 2017
@karmatosed
Copy link
Member

Removing 'needs design' as in progress :) Thanks for tackling this @notnownikki and @jwold ❤️

@afercia
Copy link
Contributor

afercia commented Aug 31, 2017

If this is going to use an "expandable div" then please see #469. Te toggle should use aria-expanded, the expandable div should be immediately after the toggle in the source order.

@karmatosed
Copy link
Member

I think rather than reopening something that has been added and works, let's create a new issue to enhance and improve it.

@karmatosed
Copy link
Member

#2667 is the new ticket.

@afercia
Copy link
Contributor

afercia commented Sep 5, 2017

Sorry to reopen, I see a few issues introduced in #2628

1
the "cog" button doesn't have any text or aria-label, it's completely empty so it will be reported by assistive technologies just as "button". Worth noting the other buttons that are very close in the code, they all have an aria-label...

Since it misses an aria-label, the button misses also a tooltip. I'd say it should have an aria-label with text "Link settings" or something along those lines.

screen shot 2017-09-05 at 16 06 11

2
z-index issue!
see screenshot above: when entering or pasting a link and there are also search results, when pressing the cog button the expanded panel stays below the search results.
To reproduce, insert a link to https://wordpress.org in a couple other posts. Then edit a third post and insert a link pasting in the field https://wordpress.org.
I'd say either increase the x-index or, maybe better, close the search suggestions when the setting panel is expanded, and display them again when the setting panel gets closed.

3
the input field misses a focus style.
This is because the input fields inherit the focus style from forms.css where input of type="url" aren't targeted. Two options here:

  • set the input field to type="text" and miss minor advantages given by type="url"
  • add proper selectors to the styles in core forms.css, targeting type="url"

4
IE 11 and Edge display a proprietary "X" control in the input fields of type="url":

ie11

edge

This would be a minor issue, but unfortunately the spinner that appears when the search runs doesn't look so nice in combination with the "X":

screenshot 62

If I remember correctly, a similar issue happened in the Customizer for the menu/widget search field and we just switched back the input field type "search" to "text" (which would also solve the focus style issue).

@afercia afercia reopened this Sep 5, 2017
@notnownikki
Copy link
Member

@afercia thank you! I'll take care of those, perhaps I could ping you for more feedback once I've got them taken care of?

@afercia
Copy link
Contributor

afercia commented Sep 5, 2017

@notnownikki sure! Agreed with @karmatosed to close this in favor of #2667 that should be updated with the right issues (sorry for the confusion).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants