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

Resolves #1139: can use a direct permalink to a specific level #1140

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

pomeh
Copy link
Contributor

@pomeh pomeh commented Apr 24, 2024

This resolves the issue #1139 I just created, about permalink to levels.

I did not add unit tests as I didn't find anywhere query arguments are tested. But I've manually tested my changes through Gitpod (which is simply amazing !).

I also was thinking about a "share level" feature, but:

  • this another feature
  • this is out of scope of the initial issue
  • this can have many implementations possible, and different UI design too
  • this idea would need your validation first, before any change

Feel free to let me know what you think about all this.

Copy link

netlify bot commented Apr 24, 2024

Deploy Preview for xenodochial-hugle-b9ec84 ready!

Name Link
🔨 Latest commit d09fc9e
🔍 Latest deploy log https://app.netlify.com/sites/xenodochial-hugle-b9ec84/deploys/6628dbd4ce888500089464af
😎 Deploy Preview https://deploy-preview-1140--xenodochial-hugle-b9ec84.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -237,6 +237,10 @@ var initDemo = function(sandbox) {
'levels'
];
commands = commands.join(';#').split('#'); // hax
} else if (params.hasOwnProperty('level')) {
commands = [
'level ' + unescape(params.level),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not implemented checks on the params.level format/value here.

I'm not really confortable with this idea, as an app must always validate input args/user values before using them, but I've not implemented it myself to stick to the coding style for others parameters, where there is no check either.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries, we do some validation when we run the command and provide an error response :)

Screenshot 2024-04-24 at 9 56 30 AM

@pcottle pcottle merged commit 03a82ce into pcottle:main Apr 24, 2024
5 checks passed
@pcottle
Copy link
Owner

pcottle commented Apr 24, 2024

Looks great! I also think this will work if you provide both a command and a level param, which is nice :) and the level one will run first too!

@pomeh pomeh deleted the 1139-add-direct-link-to-levels branch April 24, 2024 15:52
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