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

#62 - [new] Add links to titles #63

Merged
merged 3 commits into from
Oct 5, 2021

Conversation

mattwatsoncodes
Copy link
Contributor

Fix for: #62

Add the following options to render a link either as a tool tip, or inline:

'link'          => esc_url( 'https://google.com' ),
'link_type'     => 'tooltip', // Can be 'tooltip' or 'link'. Default is 'tooltip'.
'link_text'     => esc_html__( 'Learn More', 'orderable' ), // Default is 'Learn More'.
'link_external' => true, // Default is `true`.

Video demo: https://cln.sh/jLN6XC

@pramodjodhani
Copy link
Collaborator

@mattwatsoncodes I think it would be cleaner if we make link as a subarray and then have all link-related attributes inside it. Like this:

'link' => array(
   'url'           => esc_url( 'https://google.com' ),
   'type'        => 'tooltip', // Can be 'tooltip' or 'link'. Default is 'tooltip'.
   'text'         => esc_html__( 'Learn More', 'orderable' ), // Default is 'Learn More'.
   'external' => true, // Default is `true`.
),

@jamesckemp what do you suggest?

@pramodjodhani pramodjodhani requested review from jamesckemp and removed request for pramodjodhani September 24, 2021 08:57
@pramodjodhani pramodjodhani removed their assignment Sep 24, 2021
@mattwatsoncodes
Copy link
Contributor Author

I did think of doing that, but there wasn't a precedent for it elsewhere in the settings.

Happy to go with either @jamesckemp @pramodjodhani.

@pramodjodhani
Copy link
Collaborator

yes, but we recently did it here: #56

@mattwatsoncodes
Copy link
Contributor Author

In that case I'll alter it, as it is much neater that way 👍

@jamesckemp jamesckemp merged commit d70a17c into master Oct 5, 2021
@jamesckemp jamesckemp deleted the feature/62-add-ability-to-add-link branch October 5, 2021 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants