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

Product Notes: A tag to give update about a product stock #552

Merged

Conversation

patrickreiffenstein
Copy link
Contributor

@patrickreiffenstein patrickreiffenstein commented Jan 12, 2025

Picture of the new tags for reference:
image

@patrickreiffenstein patrickreiffenstein linked an issue Jan 12, 2025 that may be closed by this pull request
@patrickreiffenstein patrickreiffenstein marked this pull request as ready for review January 13, 2025 14:29
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

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

Awesome feature!

stregsystem/models.py Outdated Show resolved Hide resolved
stregsystem/templates/stregsystem/index.html Show resolved Hide resolved
stregsystem/tests.py Show resolved Hide resolved
stregsystem/tests.py Show resolved Hide resolved
stregsystem/views.py Outdated Show resolved Hide resolved
stregsystem/models.py Outdated Show resolved Hide resolved
@krestenlaust krestenlaust requested a review from atjn January 13, 2025 14:49
This includes:
- Putting filters together into a single Q statement
- More reuse of objects in product note testing
- Fix test_menu_index, which I fixed badly earlier
- Better comments for ProductNote model
@patrickreiffenstein
Copy link
Contributor Author

@VinceUB put your thoughts about the tags in here

@VinceUB
Copy link

VinceUB commented Jan 13, 2025

== English below ==
Hej Patrick,

Jeg har kigget på dine designs, og jeg har nogle forslag til ændringer, som du kan se i billedet nedenfor:

image
Ændringer:

  • Teksten er hvid i stedet for sort, da jeg føler at det gør det nemmere at læse. (Hvis en af mærkerne har gul baggrund, skal teksten være sort.)
  • Jeg har flyttet mærket til slutningen af linjen, hvilket føles mere naturligt i min mening.
  • Jeg har eksperimenteret med at køre med gul baggrund frem for mærkerne, lidt à la spotvarer på supermarkedet. Jeg ved dog ikke om jeg er helt fan, den er lidt for støjende.

Jeg har ikke ændret det i selve koden, da jeg ikke kunne få stregsystemet op at køre :(

Med venlig hilsen,
Vincent

== Dansk ovenfor ==

Dear Patrick,

I have looked on your designs, and I have some suggestions to changes, that you can see in the picture below:

image
Changes:

  • The text is white instead for black, because I feel that it does it easier to read. (If one of the labels has yellow background, should the text be black.)
  • I have moved the label to the ending of the line, which feels more natural in my opinion.
  • I have experimented with to go with yellow background instead of labels, a little à la spot products on the supermarket. I, however, don't know if I'm a complete fan, it is a little too noisy.

I have not changed it in the code itself, since I could not get the line system up to drive :(

With friendly greetings,
Vincent

@patrickreiffenstein
Copy link
Contributor Author

== English below == Hej Patrick,

Jeg har kigget på dine designs, og jeg har nogle forslag til ændringer, som du kan se i billedet nedenfor:

image Ændringer:

* Teksten er hvid i stedet for sort, da jeg føler at det gør det nemmere at læse. (Hvis en af mærkerne har gul baggrund, skal teksten være sort.)

* Jeg har flyttet mærket til slutningen af linjen, hvilket føles mere naturligt i min mening.

* Jeg har eksperimenteret med at køre med gul baggrund frem for mærkerne, lidt à la spotvarer på supermarkedet. Jeg ved dog ikke om jeg er helt fan, den er lidt for støjende.

Jeg har ikke ændret det i selve koden, da jeg ikke kunne få stregsystemet op at køre :(

Med venlig hilsen, Vincent

== Dansk ovenfor ==

Dear Patrick,

I have looked on your designs, and I have some suggestions to changes, that you can see in the picture below:

image Changes:

* The text is white instead for black, because I feel that it does it easier to read. (If one of the labels has yellow background, should the text be black.)

* I have moved the label to the ending of the line, which feels more natural in my opinion.

* I have experimented with to go with yellow background instead of labels, a little à la spot products on the supermarket. I, however, don't know if I'm a complete fan, it is a little too noisy.

I have not changed it in the code itself, since I could not get the line system up to drive :(

With friendly greetings, Vincent

Hello my dear friend

Concerning the text colors. I thought about setting giving the possibility of setting the text color for the tags, but I felt it was too much, and I feel like I can read it best with black text.

I can see that the moving of the tag to the end is a possibility, but I feel like you mostly look at the left side of the table, so I feel like it is most attention grabbing there.

I agree on the row coloring being a bad idea.

Yours truly, Patrick

@krestenlaust
Copy link
Member

== English below == Hej Patrick,
Jeg har kigget på dine designs, og jeg har nogle forslag til ændringer, som du kan se i billedet nedenfor:
image Ændringer:

* Teksten er hvid i stedet for sort, da jeg føler at det gør det nemmere at læse. (Hvis en af mærkerne har gul baggrund, skal teksten være sort.)

* Jeg har flyttet mærket til slutningen af linjen, hvilket føles mere naturligt i min mening.

* Jeg har eksperimenteret med at køre med gul baggrund frem for mærkerne, lidt à la spotvarer på supermarkedet. Jeg ved dog ikke om jeg er helt fan, den er lidt for støjende.

Jeg har ikke ændret det i selve koden, da jeg ikke kunne få stregsystemet op at køre :(
Med venlig hilsen, Vincent
== Dansk ovenfor ==
Dear Patrick,
I have looked on your designs, and I have some suggestions to changes, that you can see in the picture below:
image Changes:

* The text is white instead for black, because I feel that it does it easier to read. (If one of the labels has yellow background, should the text be black.)

* I have moved the label to the ending of the line, which feels more natural in my opinion.

* I have experimented with to go with yellow background instead of labels, a little à la spot products on the supermarket. I, however, don't know if I'm a complete fan, it is a little too noisy.

I have not changed it in the code itself, since I could not get the line system up to drive :(
With friendly greetings, Vincent

Hello my dear friend

Concerning the text colors. I thought about setting giving the possibility of setting the text color for the tags, but I felt it was too much, and I feel like I can read it best with black text.

I can see that the moving of the tag to the end is a possibility, but I feel like you mostly look at the left side of the table, so I feel like it is most attention grabbing there.

I agree on the row coloring being a bad idea.

Yours truly, Patrick

Dear @VinceUB and @patrickreiffenstein

I hope this comment, on a GitHub thread, finds you well.

What do you say to this comprimisation? Patrick can implement the "yellow rabatskilt"-feature using javascript using note tags, and he can implement a text and background color, then it's up to the staffmember to decide what to do

@patrickreiffenstein
Copy link
Contributor Author

I have now added the possibility of changing the color of the text for @VinceUB. I will not add the "yellow rabatskilt"-feature, Vincent can make some javascript for the textfield if he really wants to use it for something as Kresten said.

@krestenlaust krestenlaust changed the title Feat: Product Notes, a tag to give update about a product Product Notes: A tag to give update about a product stock Jan 14, 2025
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

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

Great work! The feature turned out more simple (code wise) than I had expected, so I'll retire the review requests.

You may merge when you feel like it 👍

stregsystem/models.py Show resolved Hide resolved
@patrickreiffenstein patrickreiffenstein merged commit d9fe7b0 into f-klubben:next Jan 14, 2025
3 checks passed
@atjn
Copy link
Contributor

atjn commented Jan 19, 2025

Tror I glemte at lave et quickbuy før I mergede det her:
image

@atjn
Copy link
Contributor

atjn commented Jan 19, 2025

Jeg har fundet et fiks: #554

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"New"-label for new products
4 participants