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

Modified quick use button #541

Merged

Conversation

SergioCasCeb
Copy link
Contributor

  • Changed the quick-use button to a simpler, more obvious variation.
  • Instead of utilizing an icon that implies import, the button now has the simple text of "use"

- Changed the quick use button to a simpler, more obvious variation.
Copy link

netlify bot commented Dec 6, 2023

Deploy Preview for thingweb-playground ready!

Name Link
🔨 Latest commit 686a8f2
🔍 Latest deploy log https://app.netlify.com/sites/thingweb-playground/deploys/659e0fbf30081900088af244
😎 Deploy Preview https://deploy-preview-541--thingweb-playground.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.

@egekorkan
Copy link
Member

@danielpeintner @mkovatsc do you prefer this more direct "USE" button compared to the icon?

@egekorkan egekorkan linked an issue Dec 11, 2023 that may be closed by this pull request
@danielpeintner
Copy link
Member

@danielpeintner @mkovatsc do you prefer this more direct "USE" button compared to the icon?

I think it is easier to understand when reading the text without hovering over.

It could even read "LOAD" to me... since I would be unsure what "USE" means... anyhow fine as is.

@egekorkan
Copy link
Member

@danielpeintner now we use load as it also makes more sense to me.
The failing test is being addressed in another PR.

@SergioCasCeb there should be no changes to the copyright years in this PR. Can you rebase it?

@SergioCasCeb
Copy link
Contributor Author

SergioCasCeb commented Jan 5, 2024

@egekorkan what exactly do you mean with the copying years?

Also should the button, when opening the example card, also say load instead of apply to avoid any confusion between the functionality of each button?

@egekorkan
Copy link
Member

Also should the button, when opening the example card, also say load instead of apply to avoid any confusion between the functionality of each button?

I hadn't seen that sadly. I think that row with two buttons can either be removed (not sure yet). For now, the "apply" button should also say load.

@egekorkan
Copy link
Member

@egekorkan what exactly do you mean with the copying years?

Currently, this PR has 97 file changes but they are due to copyright years (see screenshot below) changing.

image

It even shows the change again (not sure why) since #546 has already did the correction. We are now using the creation date instead of the current year.

@SergioCasCeb
Copy link
Contributor Author

Ok, I will change the text of the other Button.

And rebase the branch to fix the copyright texts.

- The button text has been changed from Apply to Load to match the text of the quick use button
- Changed the quick use button to a simpler, more obvious variation.
- Changed the quick use button to a simpler, more obvious variation.
- The button text has been changed from Apply to Load to match the text of the quick use button
@SergioCasCeb
Copy link
Contributor Author

@egekorkan , I fixed the button name and some styling issues. Still, regarding the copyright issue, I got a bit confused because I checked some other packages in the master branch from the eclipse-thingweb repository and they also have different dates.

By date of creation do you mean the creation of the individual package or the project itself?

Also I don't think rebasing will help me do that, but if you want, with the search function in VSCode I can set everything again to the correct date, or should that be done in another PR on itself?

@egekorkan egekorkan changed the base branch from master to advanced-tm-validation January 10, 2024 08:56
@egekorkan egekorkan changed the base branch from advanced-tm-validation to master January 10, 2024 08:56
@egekorkan
Copy link
Member

Changing the base branch twice solved the copyright year issues. Merging

@egekorkan egekorkan merged commit ef0324a into eclipse-thingweb:master Jan 10, 2024
10 of 12 checks passed
@SergioCasCeb SergioCasCeb deleted the simpler-user-button-sergio branch January 11, 2024 14:28
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