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

Improve button configuration / extensibility #10

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

LukeTowers
Copy link
Member

No description provided.

@goldmont
Copy link
Contributor

Hi,

I would suggest also this change in views/buttons/provider to allow logo removal:

<div class="winter-sso">
    <a
        href="<?= e($url); ?>"
        class="flex items-center py-0 px-4 m-0 mt-4 w-full h-12 text-center normal-case bg-transparent bg-none rounded border border-cyan-700 border-solid cursor-pointer"
    >
	    <?php if ($logoUrl): ?>
			<div class="flex items-center font-sans leading-6 text-cyan-700">
				<div class="flex justify-center items-center w-8 h-8 text-center rounded-full bg-zinc-100">
						<img
							src="<?= e($logoUrl); ?>"
							alt="<?= e($logoAlt); ?>"
							class="block w-5 max-w-full h-5 align-middle"
						/>
				</div>
				<div class="mr-0 ml-3 w-px h-6 text-center bg-gray-400"></div>
			</div>
	    <?php endif; ?>
        <div class="flex-1 font-sans leading-6 text-cyan-700 <?= $logoUrl ? '-ml-9' : '' ?>">
            <p class="m-0 font-medium leading-7 cursor-pointer">
                <?= e($label); ?>
            </p>
        </div>
    </a>
</div>

@goldmont
Copy link
Contributor

goldmont commented Jan 18, 2025

Furthermore, consider replacing Config::get('winter.sso::enabled_providers', []) with array_keys(Config::get('winter.sso::enabled_providers', [])) because now enabled_providers is an associative array.

Plugin.php:134
Plugin.php:162
controllers/Handle.php:56

Otherwise, let's create a new key... e.g. view_overrides

@LukeTowers LukeTowers changed the title Update Plugin.php Improve button configuration / extensibility Jan 21, 2025
@LukeTowers
Copy link
Member Author

@goldmont give the latest commits a try, I've moved the button config to the providers array instead of the enabled_providers so it shouldn't conflict anymore.

@goldmont
Copy link
Contributor

Hi,

It's ok to me. Consider removing the -ml-9 class from the label container to make it properly centered.

@goldmont
Copy link
Contributor

goldmont commented Jan 22, 2025

Also, is it possible to bumb the library to a stable version? In my composer.json I had to switch minimum-stability from stable to dev in order to install the library.

@LukeTowers LukeTowers marked this pull request as ready for review January 22, 2025 20:30
@LukeTowers LukeTowers merged commit bd30e54 into main Jan 22, 2025
@LukeTowers LukeTowers deleted the wip/improve-button-extensibility branch January 22, 2025 20:34
@goldmont
Copy link
Contributor

@LukeTowers There is an error in the view as I pointed out in a previous comment.

@LukeTowers
Copy link
Member Author

@goldmont which error?

@goldmont
Copy link
Contributor

The first argument of View::make should be data["view"]

@LukeTowers
Copy link
Member Author

LukeTowers commented Jan 22, 2025

@goldmont ah, thanks! Fixed in d6e998c

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.

2 participants