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

Add tailwindcss show / hide strategies #299

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

juchom
Copy link

@juchom juchom commented May 31, 2022

Following the discussion on discord, these strategies will add or remove the associated css class from tailwindcss.

@juchom juchom changed the title Add tailwindcss show / hide strategy Add tailwindcss show / hide strategies Jun 1, 2022
@1cg
Copy link
Contributor

1cg commented Jun 2, 2022

I'd like to make this an extension to hyperscript, rather than the core. Are you OK with that?

We would need to add HIDE_SHOW_STRATEGIES to _hyperscript.config so you can add to it externally.

Are you OK w/ that idea? Moving this to something like tailwinds_ext.js?

@juchom
Copy link
Author

juchom commented Jun 2, 2022

I'd like to make this an extension to hyperscript, rather than the core. Are you OK with that?

You're the boss, you tell me what to do 😄

And actually it makes sense 👍

We would need to add HIDE_SHOW_STRATEGIES to _hyperscript.config so you can add to it externally.

How can I do that ?

Are you OK w/ that idea? Moving this to something like tailwinds_ext.js?

Where am I supposed to create this file lib/plugin or lib/extensions or somewhere else ?

@1cg
Copy link
Contributor

1cg commented Jun 2, 2022

If you don't mind waiting a week, we are going to do some restructuring and can look at how things shake out after that.

@juchom
Copy link
Author

juchom commented Jun 2, 2022

If you don't mind waiting a week, we are going to do some restructuring and can look at how things shake out after that.

Ok, no problem for me.

1cg added a commit that referenced this pull request Oct 4, 2022
@1cg
Copy link
Contributor

1cg commented Oct 4, 2022

OK, yeesh, finally getting back to this.

So here is what I'd like to do: can you create a new extension in the /src/ext/ directory (doesn't currently exist):

/src/ext/tailwinds.js

In that file, you can add in these transitions with the following code:

_hyperscript.config.hideShowStrategies.twVisibility = function (op, element, arg) {
			if (op === "toggle") {
				if (element.classList.contains("invisible")) {
					HIDE_SHOW_STRATEGIES.tailwindcss("show", element, arg);
				} else {
					HIDE_SHOW_STRATEGIES.tailwindcss("hide", element, arg);
				}
			} else if (op === "hide") {
				element.classList.add('invisible');
			} else {
				element.classList.remove('invisible');
			}
		}

You'll need the latest code from dev, where I init _hyperscript.config.hideShowStrategies properly.

We can use this file as a basis for a bunch of tailwinds-specific stuff for hyperscript.

@juchom
Copy link
Author

juchom commented Oct 5, 2022

Where do you want to have the tests for the strategies ?

@1cg
Copy link
Contributor

1cg commented Oct 8, 2022

I vote /test/ext/tailwinds.js

Does that sound reasonable?

@juchom
Copy link
Author

juchom commented Oct 10, 2022

I've added the tests like this one but they all fail.

describe("tailwindcss extensions", function () {
    beforeEach(function () {
		clearWorkArea();
	});
	afterEach(function () {
		clearWorkArea();
        _hyperscript.config.defaultHideShowStrategy = null;
	});

    it("can hide element, with tailwindcss hidden class", function () {
        _hyperscript.config.defaultHideShowStrategy = _hyperscript.config.hideShowStrategies.twDisplay;
		var div = make("<div _='on click hide'></div>");
		div.classList.contains("hidden").should.equal(false);
		div.click();
		div.classList.contains("hidden").should.equal(true);
	});
});

The function to test is this one :

_hyperscript.config.hideShowStrategies.twDisplay = function (op, element, arg) {
    if (op === "toggle") {
        if (element.classList.contains("hidden")) {
            _hyperscript.config.hideShowStrategies.twDisplay("show", element, arg);
        } else {
            _hyperscript.config.hideShowStrategies.twDisplay("hide", element, arg);
        }
    } else if (op === "hide") {
        element.classList.add('hidden');
    } else {
        element.classList.remove('hidden');
    }
}

I'm missing something, but I don't get what...

@1cg
Copy link
Contributor

1cg commented Oct 8, 2023

heya @juchom I want to take a look at this PR again. Are you good w/ resurrecting it?

@juchom
Copy link
Author

juchom commented Oct 10, 2023

Ok, tell me what to do

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.

None yet

2 participants